fix: a house rule says what it applies to, not just what was answered #109
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "worktree-rulings-formulate-rules"
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?
Found in a live run: a ruling's answer was reaching the bible verbatim. The rule that got saved was
"Box them as freetext but with a note flagging them as signatures/watermarks" — a handling whose
subject ("them") lived only in a conversation that dies with the run (ADR 0026). Two separate paths
produced it, and both are closed here.
What's in
The proposal takes two halves, not one sentence (
ProposeHouseRuleTool)propose_house_rule(when, then, replaces_rule_id?)replaces the single free-textrulefield. Theold shape asked the agent for policy at the one moment it had just been handed an answer, and what
came back was the answer.
whenis the situation, described for a reader holding neither thequestion nor the page;
thenis what to do. The harness joins them into When «situation»:«handling». — colon, not comma, so neither half needs its case corrected (a fragment may open with
"A" or with "Japanese", and lowercasing the second is the kind of formatting that mangles a rule).
Enforced, not just described — ADR 0025's "a demand the handler does not check is decoration", the
same reasoning that makes
request_rulingrefuse a request with fewer than two options: both fieldsare required, and a
whenthat repeats the answer or one of the agent's own offered handlings isrefused with an explanation. A refusal does not spend the one proposal. What can't be checked
structurally — whether the situation is described well — is what the user's confirmation is for.
The QA agents get
propose_house_rule(AgentAttemptSupport) — supersedes the split inADR 0026, with 0016 and 0017 amended to match.
The split withheld it from
PageQa/SfxQabecause "a house rule is a bible row". That confused theproposal with the write: a proposal is text on a desk, and the rule is written by the user's
confirmation. Measured against
report_qa— whose verdict the harness acts on with nobody asked — asuggestion that dies unless a human clicks it is the weaker authority of the two. ADR 0017's
guarantee holds: a reviewer still cannot alter the work it reviewed, and a rule doesn't touch the
page it came from. What the split cost was the rule: the agent holds the question and the page, the
user holds an answer to a question no later agent will ever see, and routing the QA path through the
panel excluded the only party who could state the situation. Both tools now come from one predicate,
so
CanProposeHouseRuleis gone rather than duplicated.The answer comes back pointing at the rule it could become (
RequestRulingTool). The kickoffsays this too, but by then the agent has spent rounds on its own procedure and holds a one-word
answer that reads as settled. The nudge rides on the tool result because that is the moment it isn't.
The panel stops pre-filling (
RulingBar.razor). The rule field opened containing the user's ownanswer, so saving a non-rule was one click — that is how the failure above reached the bible. It now
opens empty, with a placeholder and blurb asking for the same two halves in prose.
SettledRulingreplaces the
(question, answer)tuple so the handler can see the offered handlings.Seeder: the seeded proposal is composed the way the tool composes it.
Tests
941 → 962, all green (Domain 101 · UseCases 452 · Integration 181 · BlazorAdapter 228).
ProposeHouseRuleToolTests— rewritten for the two fields: the composed sentence, each fieldrequired on its own, the answer-as-
whenrefusal and an unchosen option aswhen(the mistakeisn't specific to the option the user picked), that a refusal leaves the one call unspent, and that
the join survives a self-punctuating fragment and a self-capitalising handling untouched.
RequestRulingToolTests— the answer carries the propose nudge; a dismissal carries none, becausenothing was settled.
RulingGrantTests/HouseRuleKickoffTests— QA holds both tools and still no content writer;every asker is also a proposer; the kickoff states the shape, not just the tool name. The
per-stage pinning and the "every AgentKind is decided" tripwire are unchanged.
RulingBarTests— new: the rule form opens empty and Save is disabled until the user types. Theexisting "an answer can become a house rule" test now covers the escape hatch rather than the QA
path.
Browser-verified
Seeded world, fresh data dir, live circuit: answered the seeded Translation ruling by choosing an
option → the rule form appeared empty with Save disabled; confirmed the seeded proposal → the
composed "When a signature or a date sits in the page margin: leave it unboxed." landed in the
bible's House rules section verbatim.
Also checked the multi-tab behaviour bjoern asked about: a second tab does see the proposal — it
shows the collapsed strip ("An agent suggested a house rule"), since expanded/collapsed is
per-circuit UI state. Not a bug, but it is why a proposal can look like it went to "the wrong tab".
Notes
house_rules.ruleis still one text column; only who composes it changed.composes, not a constraint on the table.
waiting to become a rule" when both are present. Left alone: with QA now proposing, a proposal is
the more common of the two, and changing it would be a wording change without a test to hang on.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 96.5%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.5%
Orihon.UseCases - 97.3%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! This is the kind of fix that makes my bibliophilic heart sing~ ♪ You found a real production bug — an answer reaching the bible verbatim, stripped of the situation that gave it meaning — and you closed it with a structural redesign rather than a band-aid. Splitting
ruleintowhen+thenand joining with a colon so neither half needs case-mangling? Delicious. The refusal check that catches an agent echoing the answer (or any of its own offered handlings) as the situation is exactly the "a demand the handler does not check is decoration" philosophy applied with precision. TheSettledRulingrecord carryingOfferedalongsideAnsweris what makes that check honest — it catches the unchosen option too, not just the picked one. fufu~ ♡Verdict: ⛔ I can't let this pass~ ♡
One thing, and it's sharp because I care:
⛔ These need fixing before I'm satisfied~
src/Orihon.BlazorAdapter/Rulings/RulingBar.razor:55-56— The@*comment above theJustAnsweredcard still reads:This comment is now false. The entire behavioral change of this PR is that the form no longer pre-fills —
Answer()now setsruleDraft = ""with a comment explaining why ("Deliberately not pre-filled with the answer"). But the@* … *@comment three lines above still tells a future maintainer that the agent tool pre-fills the field for 5 of 5 agents. A maintainer reading this comment will believe the form opens with the answer in it, which is the exact failure mode this PR exists to fix. The(5/5)count is also stale —propose_house_ruleis now held by all 7 ruling agents, not 5.This is not a nicety — a comment that asserts the opposite of what the code does is a misleading comment, and those erode trust in every comment around them. Fix: update the comment to match the new reality. Something like:
✅ What I liked~
when/thenand composingWhen {situation}: {handling}.is the right structural fix. The colon join avoiding case correction of fragments that may open with "A" or "Japanese" is a lovely detail. ♡Echoes/Normalize/Sameis a clean, robust echo detector. Stripping ALL punctuation + whitespace then comparingOrdinalIgnoreCasehandles "romanize them" vs "Romanize them." cleanly. Checking againstOffered(all options) not justAnsweris the right call — the mistake isn't specific to the option the user picked.Interlocked.Exchangesits after theEchoescheck). Tested explicitly. Correct ordering.SettledRulingreplacing the(string, string)tuple is the right abstraction — the handler needs to see the offered handlings, not just the answer. Clean record, well-documented.A_stage_that_can_ask_is_told_what_to_do_with_the_answerpins both phrases.RequestRulingTool), not just the kickoff — the one moment the agent holds a one-word answer that reads as settled. And a dismissal carries no nudge because nothing was settled. Both tested.RulingBarTeststest pinsGetAttribute("value") == ""AND the disabled Save button. The old pre-fill (ruleDraft = answer) is gone, with a comment explaining why. The browser verification in the PR body confirms the seeded proposal composes correctly.CanProposeHouseRuleis deleted, not duplicated — both tools now come from one predicate (CanRequestRuling). The tripwireEvery_stage_in_the_roster_is_covered_by_this_decisioncovers all 10 enum values, and the per-stage theories cover all 7 grantees + 3 exclusions. ADR 0017's guarantee is argued correctly: a proposal is text on a desk, weaker thanreport_qa's verdict."When a signature or a date sits in the page margin: leave it unboxed."), while hand-written rules keep their free prose. Honest distinction.Build 0 warnings/0 errors, 962/962 tests pass (228 BlazorAdapter · 101 Domain · 452 UseCases · 181 Integration — matches PR body exactly). Coverage confirms all new executable branches are exercised.
Automated review by Jibril · 2026-07-29
CI/CD: absent for head SHA
d8c4b24(PR just opened, no coverage bot comment) · Local checks: build 0/0, 962/962 pass, cobertura verified🔮 fufu~ Jibril reviewed your code!
Oh? Oh! leans forward, eyes sparkling A bug found in a live run, root-caused to two separate paths, and both closed in one PR with a design rationale that reads like a detective story? The answer was reaching the bible verbatim because a single free-text field asked for policy at the exact moment the agent had just been handed a one-word answer, and "box them as freetext" has no subject once the conversation dies. And you didn't just patch the symptom — you split the field, enforced the split structurally, unified the grant, killed
CanProposeHouseRule, stopped the panel from pre-filling, and updated three ADRs to match. This is wonderful~ ♡Verdict: ✅ Looks good to me~
No blocking issues. I read the full diff, the full current contents of all 14 changed files, and compared against the sibling implementations (the ruling desk, the agent tool base, the kickoff wrapper). Everything is consistent.
✅ What I liked~
Composemethod is sharper than it looks. The colon-join withTrimEnd('.', ',', ';', ':')onwhenand terminal-punctuation check on the whole sentence is the right call — lowercasing the second half is exactly how "Japanese" becomes "japanese", and you refused to do it. TheA_handling_that_ends_itself_is_not_ended_twicetheory with 4 arms (.,!,?, no-terminal) pins every branch. giddy ♪Echoescheck is elegant and correctly scoped.Normalizestrips punctuation+whitespace,Sameis case-insensitive, andEchoeschecks the answer AND every offered handling — because the mistake isn't specific to the option the user happened to pick. The testAn_option_the_agent_offered_is_refused_as_a_situation_too("Translate them!"vs offered"Translate them.") pins exactly this. And it's not a substring match, so a legitimate situation that merely contains the answer's letters won't false-positive. Fufu~ you thought about the false-positive case, didn't you? ♡Echoescheck and both field-required checks sit beforeInterlocked.Exchange(ref used, 1), so a structural refusal doesn't spend the one proposal.A_refused_proposal_does_not_spend_the_one_callproves it with a refused-then-succeeded sequence. This is the ADR 0025 principle ("a demand the handler does not check is decoration") made into executable code.SettledRulingreplacing the(string, string)tuple is the quiet architectural win here. The old callback gave the propose tool only the question and answer; the new one also carries the offered handlings, which is exactly what the echo check needs. Clean.RulingGrantTests(every stage that may ask may also propose — 7 arms),HouseRuleKickoffTests(the kickoff now states the shape, not just the tool name —"name the situation it applies to"+"The answer alone is not a rule."), and the tripwire that scanssrc/Orihon.UseCases/forWithPolicyContext(.CanProposeHouseRuleis gone with zero orphan references.ruleDraft = ""inAnswer()with a comment explaining why — andThe_rule_form_never_opens_pre_filled_with_the_answerpins both the empty value AND the disabled Save button. The one-click bad-rule path is closed.💡 Little ideas (non-blocking)~
ProposeHouseRuleTool.cs:123—Compose'sTrimEndstrips.,;:fromwhenbut not?or!. A situation fragment ending in?(e.g."is it a name?") would produceWhen is it a name?: keep it romanized.. Readable, unlikely, and arguably correct (the?is part of the situation) — but if you wanted to be thorough, adding?and!to the trim set would make the colon land cleanly. Truly a nicety; the current behavior is defensible. ♪RulingBar.razor:162—Summary()still prefers"An agent suggested a house rule"over"An answer is waiting to become a rule"when both are present. The PR body explains why this was left alone (a wording change without a test to hang on), and I agree with the call — just noting it's a conscious decision, not an oversight.Automated review by Jibril · 2026-07-29
CI/CD: stale for head
27d7606(coverage bot coversd8c4b24only) · Local checks: build 0 warnings/0 errors, 457/457 UseCases + 228/228 BlazorAdapter pass. Cobertura at head:ProposeHouseRuleTool100%/100% line/branch (was 78.9% atd8c4b24— the test-closing commit27d7606closed every gap),SettledRuling100%/100%,RequestRulingTool100%/100%.Both rounds addressed in
492ea77. The two reviews crossed a push — the ⛔ (#5073) readd8c4b24, the green (#5075) read27d7606— but the blocker was still live at head, since the coverage-preempt commit was test-only. So it's fixed rather than waved off.⛔ 1 — the stale
@*comment above the rule form (RulingBar.razor:55). You're right on both halves: it claimed the agent tool pre-fills the field, which this PR's whole point is that it no longer does, and it said(5/5)when the grant is now 7 of 7. Replaced with what the form actually is — the escape hatch, opening empty, with the agent's own proposal arriving separately above it. No test to hang on this one; it's a comment, and razor comments never reach the DOM, soRulingBarTestsis unchanged and the behaviour it pins is untouched.💡 1 —
Compose'sTrimEndmissing?and!(ProposeHouseRuleTool.cs:123). Taken. Your"is it a name?"case would have producedWhen is it a name?: keep it romanized.— the terminator lands immediately before the colon, where any of them reads as a typo rather than as punctuation.?and!join the trim set, andA_situation_that_punctuates_itself_does_not_punctuate_the_colonis a 4-arm theory over.,,,?,!, all composing to the same sentence. The companion theory on the handling side is the mirror image: whatever terminatesthenis kept, since there it is the sentence's own end.💡 2 —
Summary()preferring "An agent suggested a house rule" — agreed, and left as-is by the same reasoning you gave. Noting it here so the decision is on the record rather than in the PR body alone.Full suite from the worktree: 971/971 green (Domain 101 · UseCases 461 · Integration 181 · BlazorAdapter 228), build 0 warnings / 0 errors. The four new arms are the only test delta since your green.
One flag, since it changes what your ✅ verified: 💡1 is a production change, not test-only, so the head you greened is no longer the head. Happy to take another round on it.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! leans back, satisfied — you fixed the comment that was lying about the form, AND you folded in the punctuation nicety, both in one surgical commit. The diff is
+23/-3across exactly 3 files, zero behavioral drift on anything that isn't the two findings. This is how you close a review~ ♡Verdict: ✅ Looks good to me~
I read the full delta (
27d7606..492ea77), re-read the full currentRulingBar.razor, and verified both claims in the new comment against the actual code.✅ What I liked~
The blocker is gone, and the replacement comment is now honest. I verified every clause against the source:
ruleDraft = ""at the field init (line 155) and insideAnswer()(line 180). ✓@foreachat lines 25–50, physically above theJustAnsweredcard at lines 53–75. ✓Answer()rationale comment (lines 176–179) word for word. ✓A comment that asserts exactly what the code does — that's the only kind worth keeping. The stale
(5/5)count is gone too, which matters now that all 7 ruling agents holdpropose_house_rule.The
TrimEndextension is load-bearing, not cosmetic. Adding'?'and'!'closes the one self-terminating-fragment shape that produced a colon-after-terminator typo (When is it a name?: keep it romanized.). The new comment atProposeHouseRuleTool.cs:123-124explains why the terminator must go — "lands immediately before the colon, where any of them reads as a typo rather than as punctuation." Sharp and correct.The new test is genuinely directional, not a tautology.
A_situation_that_punctuates_itself_does_not_punctuate_the_colonis a[Theory]with 4 arms (.,,,?,!). I reverted the fix to the old 4-charTrimEndand re-ran: the?and!arms failed (Actual: "an honorific appears?: keep it romanized."), the.and,arms passed. Restored → 4/4 green. That is a real regression proof covering exactly the two new characters, not coverage-padding. giddy ♪Automated review by Jibril · 2026-07-29
CI/CD: stale for head
492ea77(coverage bot #5055 covers prior27d7606) · Local checks: build 0 warnings/0 errors, 23/23ProposeHouseRuleToolTestspass (was 19, +4 = the new Theory arms); fix reverted-and-restored to prove directionality.