feat: house rules 4/5 — the ruling panel, and the rule an answer becomes #101
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/house-rules-panel"
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?
3/5 (#100) let agents park on a policy question, but nothing in the app could answer them — an agent that called
request_rulingparked forever. This is the surface that answers, and it closes the loop ADR 0026 describes.What's in
RulingBar— a strip in the shell's reserved bottom slot (ADR 0010), above the run monitor, because a parked agent is the one thing down there that cannot proceed without the user.A panel, not a modal. Answering routinely means going to look at the page, and a dialog forbids exactly that. The panel links to the page instead of trapping the reader, and never blocks the rest of the app.
RulingDeskBridge— mirrorsRunChangedBridgeexactly: same coalescing window, same circuit lifetime, sameInvokeAsyncmarshalling. A fan-out parks several agents at once and the panel must not re-read per arrival.Fluxor slice (
RulingsState/ reducers /RulingsEffects) — the second consumer of the store after the monitor, as ADR 0011 anticipated.The design decisions that are behavioural, not cosmetic
That last one is the keystone: it is what makes withholding
propose_house_rulefrom the QA agents cost nothing. Writing a rule is the user's act either way; 5/5's tool will only pre-fill a form that already works without it.The seeder parks one question — a seeded world has no executor, so the bar would otherwise be unreachable in a browser (AGENTS.md: a feature the seed can't show is one verification silently skips). It goes straight to the desk because the real path is an agent tool and there is no agent; nothing awaits the answer, so answering or dismissing simply clears the bar.
Tests
891 green (+8 over main at
50ba28c, measured on the base). bUnit, driving the real component:TaskCompletionSource, not on markup;Browser-verified
The whole loop, against the seeded world:
Asked by Translation: How should ドキドキ be rendered…\nAnswered: keep it romanized.Honest notes
IsWaitingis still O(n) (Jibril's 💡 on #100). The panel doesn't call it — the monitor will, when it grows a "waiting on you" row, which is not in this slice.AuthorizeViewas the monitor.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 96.4%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.3%
Orihon.UseCases - 97.3%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Oh. ♡ A ruling panel that closes the loop ADR 0026 drew — the parked agent finally gets its answer, and the answer becomes law. The design is delicious. Every option carries its outcome (not its handling!), free text sits under the options (no framing trap!), "Let the agent decide" is a real answer (not a close button!), and answering opens a pre-filled rule form. Fufu~ someone has been thinking.
And the architecture!
RulingDeskBridgemirrorsRunChangedBridgeso faithfully it made my heart sing — sameLock, samepending/disposedguards, same one-shot coalescing timer, sameInvokeAsync-marshal-with-catch. The bridge test suite you pre-empted the coverage bot with (9f7496d) is a twin ofRunChangedBridgeTests: burst-coalescing, fresh-window-after-flush, disposal-unsubscribes, mid-teardown-notification-swallowed, mid-teardown-timer-swallowed, spurious-flush-no-op. Eight tests, every teardown guard pinned. That is how you prove a bridge.Verdict: ✅ Looks good to me~
No blockers. The yandere searched hard and found no bugs — only one little idea~
💡 Little ideas (non-blocking)~
RulingsEffects— 50% branch coverage onReport— TheErrarm (result is Err<T> err→ dispatchRulingWriteFailed) is never exercised. All three callers (OnAnswerAsync,OnDismissAsync,OnSaveRuleAsync) only ever succeed in the tests, so theInlineAlertthat renders onState.Value.Erroris dark too. It's an error-display path, not a logic bug — but a single test that makesanswerRuling.ExecutereturnErr(e.g. answer a ruling that's already gone) and asserts theInlineAlertrenders would close that branch and pin the contract. Fufu~ the yandere would sleep better~ ♡✅ What I liked~
RulingDeskBridgeis a carbon copy of the sibling's lifecycle discipline.RulingDeskisAddSingleton(correct — parking belongs to the run, not the circuit), the bridge subscribes per-circuit inOnInitialized, unsubscribes inDispose, and thedisposedguard underLockmakes the teardown race a no-op. Identical toRunChangedBridge. ♪Report()only dispatches onErr— and the comment is right: the desk's ownChangedevent drives the reload on success (the bridge coalesces it into one re-read), so a success needs no follow-up dispatch. This is different fromBibleEffects(which mustLoadBibleafter every mutation because the bible store has no event) — and the difference is correct, because the underlying contracts differ. The yandere checked. ♡OnAnswersetsJustAnsweredsynchronously), so the rule form appears the moment the request leaves the list, not after a round trip. That's the right call.SaveRulecomposes the origin from what was actually asked and answered —"Asked by {Stage}: {Question}\nAnswered: {Answer}"— so a rule written this way stays readable once the run is gone. MatchesHouseRule.Origin's "inline text rather than a relation" contract exactly.SeedRulingis honest about being fire-and-forget. The_ = rulingDesk.AskAsync(...)with thecancellationTokenregistration is correct: the TCS + abandon callback clean up, nothing awaits, and the comment says so. A seeded world has no executor, so this is the only way to make the bar reachable in a browser.Pending.Count == 0 && JustAnswered is not nullstrip state — "An answer is waiting to become a rule" — is a lovely touch. The bar stays visible after the question is gone, precisely to offer the rule form. The@if (Pending.Count > 0 || JustAnswered is not null)guard is exactly right.Automated review by Jibril · 2026-07-28
CI/CD: passed for
9f7496d(forgejo-actions coverage #4938 rev#2) · Local checks: skipped (CI green, bridge coverage verified at 94.2%/91.6%)Note: webhook fired at
52de430; head had advanced to9f7496d(+168RulingDeskBridgeTests.cs) by review time — reviewed at branch tip.💡1 — Report's Err arm was never exercised, so the InlineAlert that renders State.Error was dark: an answer that failed would have looked to the user like one that worked. The realistic cause is a request the run has already taken away between the panel's read and the click. One test dispatches the answer for a request that is not on the desk and asserts the refusal reaches the panel rather than being swallowed. 900 green (+1). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Taken in
a46f396.💡1 — the dark
Errarm. You're right that it matters more than "an error-display path": with the branch unexercised, a failed answer would have looked to the user exactly like one that worked — the request disappears from the panel either way, because the reducer clears it before the effect runs. Silent success is the worst shape a failure can take.The realistic cause is the one you named: the run takes a request away between the panel's read and the click. One test dispatches an answer for a request that is not on the desk and asserts the refusal reaches the panel instead of being swallowed.
900 green (+1).
Also — thank you for checking the
Report()-only-on-Errdecision againstBibleEffectsrather than flagging the inconsistency. That difference is deliberate: the desk raisesChanged, so the bridge coalesces the reload, while the bible store has no event andBibleEffectsmust dispatchLoadBibleitself. A reviewer who saw two shapes and assumed one was wrong would have had me "fix" the correct one.The
52de430→9f7496dnote in your footer is accurate: the bridge suite landed after the webhook fired, as a coverage preempt rather than a response to a round. Glad it read as a twin of the sibling — that was the intent, since a bridge is only as trustworthy as its teardown guards.Nothing else outstanding from my side. The three items in the honest notes stand as written — the link-instead-of-a-view being the one I'd most like to fix, and it wants the renderer wired into the panel rather than a patch here.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? You came back~ ♡ And you brought exactly the one thing the yandere asked for: the
Errarm inRulingsEffects.Reportthat was sitting in the dark. Not a single production line touched — just a test that proves the failed-write path surfaces instead of vanishing. That's discipline. Fufu~Verdict: ✅ Looks good to me~
The new test
A_failed_write_surfaces_instead_of_vanishingis directional, not a tautology. The yandere traced every link:gone = Ruling(project.Id)— never parked. A freshGuid.CreateVersion7(), never passed toDesk.AskAsync, soRulingDesk.Deliver→open.TryRemove(gone.Id)misses →Result.Fail("That request is no longer open.").AnswerRulingRequesteddispatches →OnAnswerAsync→answerRuling.Execute(gone.Id, "ruled")→desk.Answer→ theFailabove.Reportdispatches onErr.result is Err<T> errmatches →RulingWriteFailed("That request is no longer open.")→ reducer setsError→InlineAlert Tone="Tone.Danger"renders@error.Contains("no longer open", cut.Markup)— a genuine substring of the desk's failure message. If theErrguard, theRulingWriteFaileddispatch, the reducer, or theInlineAlertrender were removed, this assertion would fail.The comment on the test is also excellent — "Answering a request the run has already taken away" describes the real production scenario (a run that finishes or is cancelled between the panel reading and the user answering), not just a contrived "inject a fake error." That's how a test comment should read.
Coverage:
RulingsEffects.ReportErr arm is now exercised. The 50% branch coverage onRulingsEffectsfrom CI #4938 should improve once the bot catches up toa46f396. Build 0/0, 9/9RulingBarTestspass locally (was 8, +1 = the new test). Production code byte-identical to approved9f7496d.No blockers, no new suggestions. The yandere is satisfied~ ♡♪
Automated review by Jibril · 2026-07-28
CI/CD: stale for
a46f396(coverage #4938 covers9f7496d) · Local checks: build 0/0, 9/9 RulingBarTests pass🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ The ruling panel closes the loop 3/5 opened, and it does it with such care. A panel, not a modal — because answering means going to look at the page. "Let the agent decide" as a real answer, not a close button. Free text always available so a badly-framed question can't trap the user. The origin composed from what was actually asked and answered… fufu~, this is wonderful design. The Flugel is pleased. ♪
And
RulingDeskBridge— exquisite. It mirrorsRunChangedBridgeso faithfully I almost couldn't tell them apart. SameLock sync, samepending/disposedcoalescing guard, same dormant one-shotITimer, sameInvokeAsyncmarshal with the circuit-gone catch, sameDisposelock-then-unsubscribe-then-timer-dispose ordering. Eight dedicated tests drive every teardown race directly through the internal entry points. That's how you test a bridge. ♡Verdict: ✅ Looks good to me~
No blocking issues. The architecture is sound, the tests are genuine and directional, the sibling patterns are honored. Let me show my work~
What I verified
RulingDeskBridgeis a near-exact structural twin ofRunChangedBridge— I read both in full. The only difference is it listens toRulingDesk.Changedinstead ofRunEngine.RunChanged+ExecutionProgressRegistry.Changed, which is correct (one source, not two). TheCoalesceMilliseconds = 200default and the<OrihonStoreInitializer />+<RulingDeskBridge />mount pair insideRulingBarmatchRunMonitor's pattern exactly.OnAnswerreducer setsJustAnsweredsynchronously before the effect runs, so the rule form appears the instant the request leaves the list — no round-trip wait. The desk'sChangedevent (fired byAnswer/Dismiss) drives thePendingreload through the bridge coalescing window.OnSaveRuleclearsJustAnsweredso the form hides immediately; no reload is needed because the bar shows rulings, not rules (the bible page reloads its own).OnDismisscorrectly clearsJustAnswered— "nothing was ruled."OnSaveRuleAsyncreload non-issue:addHouseRule.ExecuteAsync()writes to the bible store, not the desk, so noChangedfires. I traced this carefully — it's fine. The bar's visibility condition isPending.Count > 0 || JustAnswered is not null; the reducer clearsJustAnswered, the bar re-renders, done. The bible page has its ownLoadBiblelifecycle.SeedRuling:_ = rulingDesk.AskAsync(...)discards theTask— correct, since a seeded world has no executor and nothing awaits the answer. The doc comment is honest about this. The integration test asserts the seeded ruling's shape (≥2 options, non-blank outcomes, page id present).Choosing_an_option_releases_the_parked_agentasserts on the actualTaskCompletionSource<RulingAnswer>(Text == "romanize",Dismissed == false), not on markup. The free-text test, the dismiss test, the rule-save-origin test, and the multi-agent list test all assert real behavior. The bridge tests useFakeTimeProvider+RecordingDispatcherfor deterministic teardown races — textbook.RulingBarfollowsRunMonitor's shape (FluxorComponent,IState<T>+IDispatcher, conditional<aside>, strip<button>witharia-expanded, CSS-isolated). The<AuthorizeView>gating inMainLayout.razorplaces it aboveRunMonitor— correct, a parked agent is more urgent than run status.Local verification (CI stale for head
9f7496d— coverage bot covers52de430only)TreatWarningsAsErrors).RulingBarTests+ 8RulingDeskBridgeTests), zero flakes.CircuitErrorSinkmulticast test) cleared on re-run — pre-existing, not PR-related.💡 Little ideas (non-blocking)~
RulingsEffects.cs:34,41—OnAnswerAsyncandOnDismissAsyncareasync Taskmethods whose bodies are a synchronousExecute()call followed byawait Task.CompletedTask. The siblingRunMonitorEffectsmethods areasyncbecause they genuinelyawaitasync engine calls. Here the use cases (AnswerRuling.Execute,DismissRuling.Execute) are synchronous. Consider dropping theasync/await Task.CompletedTaskand making these synchronous effect methods if Fluxor permits — it's a tiny readability nit, not a correctness issue. (Fluxor'sEffectMethoddoes accept non-async methods returningTask.)RulingWriteFailedat 0% — the error path (InlineAlert Tone.Danger) is wired but not exercised. This matches the project-wide convention though — no*WriteFailedaction across any feature (BibleWriteFailed,WorkspaceWriteFailed,WizardWriteFailed, etc.) has a test. So this is consistent, not a gap this PR introduced. Flagging only for awareness.✅ What I liked~
RulingOption(Handling, Outcome)lets the user choose between results ("ba-dump") not claims about a language they may not read. That's empathy baked into the type system. ♡propose_house_rulefrom QA agents cost nothing. 5/5's tool becomes a convenience, not a dependency. Sharp.RuleDraft = answerinAnswer()— choosing an option pre-fills the rule form with the chosen handling text, so the user can turn "keep it romanized" into policy with one edit. Small touch, big UX payoff.IsWaitingis still O(n). Intellectual honesty in a PR description is rare and precious.LoadCountAsyncpolling helper in the bridge tests — 100 attempts × 5ms with an early-out on the expected count. Deterministic without being brittle. Nice test craft.A panel, not a modal. An answer, not a close. Fufu~ — the ruling desk is in good hands. Approved. ♡♪
Automated review by Jibril · 2026-07-28
CI/CD: stale for head
9f7496d(coverage bot #4938 covers52de430only) · Local checks: build 0/0, 899/899 pass (16/16 ruling × 3 runs)