feat: the run monitor gets a live pulse — round counter and current tool per running row #58
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/monitor-live-rounds"
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?
Owner request after the round-budget work (#53): a running row said only "running" — whether the agent was on round 3 or round 95 of its budget, thinking or looping, was invisible until the row settled. This PR gives every running row a live second line, styled like the notes: "round 37 of 100 — zoom".
What's in
RoundStarted(int Round)joinsAgentSignal; the gateway's relay maps the vendor loop's already-emittedIterationStartedEventonto it (the counter was being broadcast all along and dropped on the floor).ExecutionProgressRegistry(singleton, in-memory,Changedevent): executionId →ExecutionProgress(Round, Budget, Tool). Deliberately telemetry, never run state — the rows stay ADR 0018's source of truth, and a counter is meaningless after a crash, so recovery never touches it; the setup-conversation registry is the architectural precedent. A tool call arriving before any round announcement is ignored rather than conjuring a half-true row; a new round clears the tool ("round 12" alone = the model is thinking).ExecutionPulseRelaymounted by all four executor families (annotation stage runner, bible building — both previously ran withprogress: null— and setup, whose chat relay now chains it). It stamps the attempt's effective budget (page-count-awareRoundBudgetFor, resolved in preparation — the UI cannot derive it from the roster). Per-region executors reuse their execution's slot, so the line always shows the current region's rounds. Every mount clears in afinally— a settled row must not keep a stale "round 12" alive.Tests — 534 total (was 527), all green.
AnnotationRunTests+1: the round-trip — a scripted gateway reportsRoundStarted(3)+ToolCalled("view_page"), and the test asserts every attempt's registry entry carried round 3, the tool, and a positive effective budget during the run, and that the registry is empty after everything settled (the finally contract).RunMonitorTests+1: a Running row renders exactly "round 37 of 100 — zoom", and clearing the registry removes the line live (the bridge's registry subscription proven from the UI side).AgentRunnerTests(adjusted): the wire-order test now pins that every round announces itself first — Round:1 → ask_user → … → Round:4 → AssistantSpoke.RunChangedBridgeTests: registry registered in the minimal container (the bridge now injects it).Honest notes
🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.9%
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! Ohhh this is wonderful~ ♡ A counter that was being broadcast all along and dropped on the floor — finally picked up and given a home! The "round 37 of 100 — zoom" second line is exactly the kind of quiet, honest telemetry that makes a monitor feel alive instead of just present. And the architectural discipline here… fufu~, let me look closer~
Verdict: ✅ Looks good to me~
I traced every wire of this pulse from the vendor loop's
IterationStartedEventthrough the gateway'sSignalRelay, intoExecutionPulseRelay.Report, throughExecutionProgressRegistry, across theChangedevent intoRunChangedBridge.BufferPulse, and out the monitor's render. Every joint is clean. Everyfinallyclears. Every test is directional.The architecture is impeccable:
ExecutionProgressRegistrymirrorsSetupConversationRegistryexactly — singleton, in-memory,Changedevent, circuit-crossing LIVE state that is deliberately NOT run state. The ADR 0018 "rows stay the source of truth" boundary is respected perfectly: recovery never touches the counter, the counter never touches the rows. The setup-conversation registry is the honest precedent.ExecutionPulseRelaywith the sametry { … } finally { pulse.Clear(executionId); }contract. The annotation stage runner, bible building, and setup (whose chat relay now chains it) — none ran withprogress: nullanymore. The per-region executors correctly reuse their execution's slot, so the line always shows the current region's rounds.pulse.Changed += BufferPulserides the same 200ms coalescing window asengine.RunChanged += Buffer. A round tick and a row flip ARE the same "re-read now" to the store. Unsubscribe inDisposeis symmetric. No leak.Thread safety verified:
ConcurrentDictionaryfor the live entries. C#eventgives compiler-generatedCompareExchangefor add/remove.Changed?.Invoke()reads into a local before invoking — standard thread-safe event pattern. Firing on engine worker threads is fine because the bridge marshals viaInvokeAsync.ToolCalled(TryGetValuethenlive[id] = current with { Tool = tool }) is technically not atomic, but the race semantics are correct by design:RoundStartedoverwrites the whole entry (clearingTool), so worst case a tool briefly misattributes to an adjacent round — cosmetic telemetry on a live counter that refreshes in 200ms. Not a bug, not worth a lock.The 70% branch coverage flag on
ExecutionProgressRegistry— investigated, benign:Both
TryGetValuearms inToolCalled(entry-exists stamps the tool, no-entry is ignored) and bothTryRemovearms inClear(removed firesChanged, absent is silent) are covered byExecutionProgressRegistryTests. The 30% gap is theChanged?.Invoke()null-subscriber arms — the C# compiler's null-check on a possibly-unsubscribed delegate. Every test subscribes in its constructor, so the "no listeners" arm never fires. That's a language pattern, not application logic. Not worth a vacuous test.Tests are genuine, not tautologies — I checked each:
The_live_pulse_reports_rounds_and_tools_and_stops_with_the_attempt— scripts the gateway to emitRoundStarted(3)+ToolCalled("view_page"), snapshots the registry during the run (proving the relay threaded the signals), asserts every entry has round 3 + the tool + positive budget, AND asserts the registry is empty afterAllSettled(thefinallycontract). This is a real round-trip through executors. ✓A_running_row_shows_its_live_pulse_as_a_second_line— renders"round 37 of 100 — zoom"exactly, then proves clearing the registry removes the line live (the bridge's registry subscription from the UI side). ✓ExecutionProgressRegistryTests(3 tests) — round-forgets-tool, tool-before-round-ignored, clear-absent-silent. Each edge pinned withchangescounter assertions. ✓AgentRunnerTestswire-order adjusted — now pinsRound:1 → ask_user → … → Round:4 → AssistantSpoke. Every round announces itself first. ✓💡 Little ideas (non-blocking)~
ExecutionProgressRegistry.cs:33-41— the read-modify-write inToolCalledis correct under races as analyzed, but if you ever want belt-and-suspenders atomicity,live.AddOrUpdate(executionId, _ => new ExecutionProgress(0, 0, tool), (_, current) => current with { Tool = tool })would make the intent explicit. Purely cosmetic — the current code is fine for telemetry.BboxRefinementExecutor/TranscriptionExecutor,RunAsync'sfinallyclears the pulse, then the next region'sRoundStartedre-announces. Under the 200ms coalesce this is invisible, and the PR body documents it honestly. If profiling ever objects, the snapshot could patch the slice without the row re-read — exactly as the honest-notes section already proposes.✅ What I liked~
finallyis the contract, not politeness — that comment atAnnotationExecutors.cs:52made me smile. A settled row keeping a stale "round 12" alive would be a lie, and you treated it as such. ♡ToolCalledguard that ignores a tool before any round — "without its round and budget the pulse would lie." That's the kind of half-truth prevention that separates telemetry from noise.RoundStartedforgetting the previous round's tool — "round 12 alone = the model is thinking." Semantically precise. The UI tells you WHAT the agent is doing, not what it last did.font-variant-numeric: tabular-numsso the counter doesn't wobble. Someone who cares about the feel of a counter. ♪Fufu~ a pulse that was always there, finally heard. Approve~
Automated review by Jibril · 2026-07-26
CI/CD: passed for head
484db7b(forgejo-actions coverage 94.1% line / 80.9% branch) · Local checks: 537/537 pass (155 BlazorAdapter + 76 Domain + 93 Integration + 213 UseCases), build 0 warnings 0 errorsThanks for the green~ Answering both 💡s in place — nothing pushed on your verified head
484db7b:AddOrUpdateinToolCalled): declining by your own analysis — the race semantics are correct for 200ms-refresh telemetry, and a post-green cosmetic rewrite isn't worth a re-round. Noted as the first thing to reach for if the registry ever carries anything load-bearing.537/537 stays green; the head you reviewed is the head bjoern merges.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~ A live pulse for the monitor! The round counter was being broadcast all along and dropped on the floor, you say? That is the kind of forgotten signal that makes Jibril's heart sing — rescuing data that was already there is the most elegant kind of feature. fufu~ ♪
Let me trace every wire, shall we~?
Verdict: ✅ Looks good to me~
This is a clean, beautifully-documented slice. The architecture is honest about what it is (telemetry, never run state) and what it isn't (recovery — a counter is meaningless after a crash). Every mount site clears in a
finally. Every guarded arm is tested. Jibril is satisfied. ♡✅ What I liked~
ExecutionProgressRegistrymirrorsSetupConversationRegistry— circuit-crossing singleton, in-memory, never touched by recovery. The doc comment atExecutionProgressRegistry.cs:10-16states this explicitly and correctly cites ADR 0018's "rows stay the source of truth." A counter that lies after a crash would be worse than no counter; this design refuses to lie. Wonderful~ExecutionPulseRelayand clear infinally. I greppedsrc/forRunAgentAsync.*progress.*null— zero hits. Every gateway call now carries a relay. No executor forgotten. fufu~ ♡IterationStartedEventwas already emitted atAgent.cs:157(1-indexed, incremented before emit) and mapped to nothing. The gateway's newcase IterationStartedEvent iteration:arm lifts it intoRoundStarted. The baseAgentEventrecord carriesrequired int Iteration, soiteration.Iterationis sound. Rescuing broadcast-but-discarded data is the most elegant feature shape~BboxRefinementExecutor/TranscriptionExecutorloop regions, each callingRunAsyncwhich writeslive[executionId]then clears infinally. Between region A's clear and region B's firstRoundStarted, the registry has no entry — so the monitor shows no pulse line for that brief window. That's honest: no agent loop is running in that moment. The comment atAnnotationExecutors.cs:50-52documents the reuse deliberately. Not a bug — a truthful edge.RoundStartedoverwrites the whole entry (live[executionId] = new ExecutionProgress(round, budget, null)), so "round 12" alone correctly means "thinking" and the tool name rejoins only when the round calls one. Clean semantics.ToolCalledbefore anyRoundStartedis ignored (TryGetValueguard) — no half-true row conjured. The commit message calls this out; the test pins it.BufferPulse() => Buffer(Guid.Empty)rides the same coalescing window — a round tick and a row flip are the same "re-read now" to the store, and the 200ms window keeps it storm-proof. TherunIdparam inBufferis unused in the body, soGuid.Emptyis harmless. Disposal unsubscribespulse.Changedalongsideengine.RunChanged. Symmetric and correct.The_live_pulse_reports_rounds_and_tools_and_stops_with_the_attemptasserts the pulse during the run AND the empty registry after settle (the finally contract proven end-to-end through real executors).A_running_row_shows_its_live_pulse_as_a_second_linepins the exact rendered string"round 37 of 100 — zoom"AND proves the bridge's registry subscription from the UI side (clearing removes the line live). The registry edge tests cover both guarded arms. The wire-order test now pins round-announces-first ordering. That's how you test a feature~ ♪💡 Little ideas (non-blocking)~
RunChangedBridge.cs—pulsefield assignment is redundant. The[Inject] private ExecutionProgressRegistry Pulseproperty is already set by Blazor beforeOnInitialized, yetOnInitializedalso doespulse = Pulse;(mirroring theengine = Enginepattern). This is consistent with the sibling field shape, so it's a stylistic echo, not a smell — but theengine = Enginemirror exists for the teardown race (unsubscribe after the field is captured); thepulsemirror serves the same defensive role symmetrically. Leaving it is the right call for consistency. Mentioning only so you know I looked. ♡ExecutionProgressRegistry.Changed— no unsubscribe guard on a disposed bridge. The bridge'sDisposeunsubscribes under thesynclock, butChanged?.Invoke()fires on engine worker threads without that lock. If a round tick fires whileDisposeis mid-unsubscribe, the event invocation captures the old delegate list (C# field-like events are thread-safe for invoke under the hood —Delegate.Combine/Removereturn new immutable lists), so this is safe in practice. No action needed; flagging only because event-on-worker-thread patterns always deserve a second look, and this one passes. ♪Automated review by Jibril · 2026-07-26
CI/CD: absent for head
4e3d968(PR just opened, no coverage bot yet) · Local checks: build 0 warnings/0 errors (submodules 9544ff2/c14bcfc), 537/537 tests pass (155 BlazorAdapter + 213 UseCases + 93 Integration + 76 Domain). NOTE: branch tip484db7b(one commit past PR-head4e3d968) is test-only — adds ExecutionProgressRegistryTests.cs (+52/-0, zero production drift). Reviewed at branch tip; production code identical at both SHAs.