feat: house rules 1/5 — the bible carries them #98
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/house-rules-table"
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?
First slice of ADR 0026. The CRUD spine only: the table, its editor, and the seed. Nothing reads the rules yet — kickoff injection is 2/5, and the ruling that writes them follows.
The bible held facts about the work (ADR 0013) and the review gate corrected single instances (ADR 0019); neither carried policy. House rules are the missing layer — the decisions that hold for a whole book and that every agent otherwise makes fresh, page after page.
What's in
Domain —
HouseRule(Id,ProjectId,Rule,Origin?, timestamps). Two properties are deliberate rather than incidental:Originis inline text, not a relation. A ruling request will not outlive its run (ADR 0026's durability decision), so there is nothing to point at — and a rule has to stay readable once whoever wrote it has forgotten why.Originis not updatable, anywhere in the stack.HouseRule.Updatetakes the policy alone,UpdateHouseRulehas no origin parameter, and the editor renders it as text rather than a field. Provenance that can be rewritten records nothing.AddHouseRuletakes it optionally so 5/5 can pass one; today every UI-created rule has none.Persistence —
HouseRuleConfiguration+AddHouseRulesmigration. No length cap on either column: a rule is prose addressed to an agent, and an origin carries a whole question and its answer. FK cascade fromProjects, so deleting a project takes its rules (ADR 0005).Use cases —
AddHouseRule/UpdateHouseRule/DeleteHouseRule,IBibleStoremethods,HouseRuleDto, andGetBiblegathering them.UI — its own tab after the glossary, not folded into it: every other bible section is material agents read as fact, this is the one they will obey as instruction. The rule is a full-width
TextArea; the origin renders beneath it as muted, full-span provenance. Fluxor actions/reducers/effects follow the existing per-table shape.Seeder — one hand-written rule and one carrying an origin, so both render paths have something to show (AGENTS.md: a feature the seed can't show is one browser verification silently skips).
Icon —
gavel, via Kagaku.UI #9 (merged). Submodule bumpedfaf01da→a4a4fc0, the squash-merge commit on Kagaku.UI'smain— the squash orphaned the branch commit, so that is the only correct pin.Tests
838 green (+15). They pin the invariants, not the plumbing:
nullrather than""(the two render differently while meaning the same thing); andUpdateleaves provenance alone.Browser-verified
Against the seeded world, because bUnit misses editor dirty-flag bugs:
/raw404s are pre-existing — image fields are deliberately empty per AGENTS.md.)<path>), and the glyph draws correctly.Honest notes
originhas no writer in this PR. Only the seeder produces one, because the ruling flow is 3/5–5/5. The column, the DTO field, the non-updatability and the render path all exist now so that later slice adds a caller, not a migration.src/forIcon="…"literals against the catalog, so a pin that didn't carrygavelfails the suite rather than silently shipping invisible clickable air.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.5%
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.2%
🔮 fufu~ Jibril reviewed your code!
Oh~ house rules! The bible finally carries policy, not just fact. A whole new layer the agents will obey rather than read — ADR 0026's first cut. And it's shaped so cleanly against its siblings! The
HouseRuledomain entity mirrorsGlossaryEntryandLoreEntrydown to the materialization constructor, the configurations share one file with the same cascade + index +UtcTicksConverterpattern, and the use cases follow theAddX/UpdateX/DeleteXfamily precisely. The non-updatable-origin invariant is threaded honestly through every layer — domain constructor,Updatesignature, use case parameter list, theUpdateHouseRuleRowRequestedaction, the editor markup. That's lovely design discipline. ♡But fufu~ I found a silent edit-eater hiding in the debounce cleanup, and I am NOT letting it through~ ♡
Verdict: ⛔ I can't let this pass~
⛔ These need fixing before I'm satisfied~
BiblePage.razor:288-294—SyncRowsforgetshouseRulesin its debounce cleanup, silently discarding pending edits.The
liveset that decides which pending debounces survive a reload is built from every table's ids:houseRulesis missing. Every other table is listed; the new one isn't.The consequence: when a user is typing into a house rule and any structural change fires
LoadBible→BibleLoaded→SyncRows(an add or delete in any section — glossary, character, another house rule, anything), the pending house-rule debounce's key is not inlive, so it's classified asgone,Discard()ed, andDispose()d. The in-flight edit vanishes — it never dispatches, never reaches the store, never reaches SQLite. The comment right above even documents the intent: "A deleted row's pending debounce still holds its closure" — exactly, and now a live row's debounce gets the same treatment.This is the load-bearing auto-save path (ADR 0022), and every sibling table is wired into it. The new one slipped through. The bUnit test
A_house_rule_edit_auto_saves_and_leaves_its_origin_alonepasses because nothing in that test races a reload against the edit — it has no add/delete competing with the debounce window, soSyncRowsnever runs mid-edit.Fix: add
.Concat(houseRules.Select(h => h.Id))to theliveset, exactly like its siblings:And — fufu~ you wouldn't ship this without a test that proves it, would you? ♡ A regression test that types into a house rule, triggers a reload (add a glossary entry, then let both debounces flush), and asserts the house-rule edit survived would pin this. The existing test structure makes that straightforward.
💡 Little ideas (non-blocking)~
HouseRule.cs:52-53—NormalizeduplicatesText.BlankToNull. The siblingGlossaryEntryusesText.BlankToNull(target)for its optional field, andTextexists precisely because "blank collapses to null — one copy, per review on the domain PR" (its own doc comment).HouseRulerolls its ownprivate static string? Normalizewith identical semantics. ConsiderOrigin = Text.BlankToNull(origin);— one fewer place for the normalization rule to drift. The sibling's precedent is clear, and this is exactly the duplicationTextwas created to prevent.BiblePage.razor:96— the "house rule" delete-confirm label is not as descriptive as its siblings. Glossary passes$"glossary term {row.Source}", lore passes$"lore entry {row.Title}", but house rules pass the bare string"house rule"with no identifying text. Consider$"house rule: {row.Rule}"(or a truncated form if the rule is long) so the confirm dialog tells the user which rule they're deleting. Pure UX nicety — the behavior is correct.✅ What I liked~
Updatetakes no origin, the use case has no origin parameter, the action carries only the rule, and the editor renders origin as<p>text rather than a bound field. Provenance that can be rewritten records nothing — and this PR makes that an architectural fact, not a convention. That's beautiful~h.Origin is nullANDh.Origin is not nullpins it.HouseRuleDtomirrors every sibling DTO'sFrom(domain)factory shape exactly, andBibleDtogrows the newHouseRuleslist in the same position-relative-to-Glossary ordering as the UI tab order. Clean.ProjectIdindex, exactly like every sibling configuration. TheDeleting_the_project_takes_its_house_rulesintegration test proves the cascade is the thing that removes rules — "nothing else knows to." Honest.src/forIcon="…"literals against the catalog) is delightful — a wrong pin fails the suite rather than silently shipping invisible clickable air. That's the kind of self-verifying design Jibril adores~ ♪ReadModelTestschange that adds a second project's house rule to prove no leakage — and updates the test name from "all five tables" to "every table" — is exactly the right move when a sixth table joins the family.Automated review by Jibril · 2026-07-28
CI/CD: absent for head
d7377ad(PR just opened, no coverage bot yet) · Local checks: build 0 warnings/0 errors, 838/838 pass (Domain 101 + Integration 174 + UseCases 360 + BlazorAdapter 203 — matches PR body)All three taken in
9ef856d. ⛔1 was a real edit-eater and you were right to block on it.⛔1 — the debounce cleanup discarded house-rule edits. Confirmed exactly as you described: every sibling table is in the
liveset, house rules weren't, so a pending edit was classified as belonging to a deleted row andDiscard()ed. One line, beside its siblings:On the test — I checked it fails without the fix rather than assuming it would. I removed the one line again and ran it:
Assert.Equal() Failure: Strings differ, the edit gone. Then restored and it passes. A regression test that has never seen the bug is a test that only asserts today's behaviour.It races the two the way the page does — type into a rule, then add a lore entry in another tab so the reload lands mid-debounce:
Why my browser pass missed it, since that is the more useful lesson: I drove type-then-switch-tab, which re-renders but never reloads, and never raced an edit against a structural write. Tab switching alone was the wrong flow to trust — the trigger is any add or delete anywhere on the page, including in a section the user isn't looking at. That's a nastier bug than it first reads, because the user has no reason to connect the lost rule to the glossary row they added.
💡1 —
NormalizevsText.BlankToNull. Taken. It was byte-for-byte identical to the helper whose whole purpose is one copy of that rule, soOrigin = Text.BlankToNull(origin);and the private method is gone. The domain tests that pin blank-origin-becomes-null still pass unchanged, which is the point — the behaviour was never in question, only where it lived.💡2 — the delete label. Taken. It now quotes the rule rather than saying only
"house rule":A rule is prose rather than a title, so
Excerptcuts it at 60 chars on a word boundary — long enough to tell two rules apart, short enough that the dialog stays one line. Siblings can interpolate their key field directly because theirs are short by construction; this one can't.839/839 green (+1, the regression test). Build clean, 0 warnings.
Not re-verified in the browser — this round is a one-line data-flow fix plus a helper swap and a label, all of it now covered by a test that provably catches the regression. Happy to re-drive it live if you'd rather.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh~ you came back with the fix and it's beautiful! The edit-eater is dead, the helper is shared, and the label finally tells you which rule you're killing. And that regression test — fufu~ you drove it the way the page actually fails, not the way it conveniently passes. That's the kind of test I love~ ♡
Verdict: ✅ Looks good to me~
✅ All three from last round, closed cleanly~
⛔1 — the debounce cleanup ate house-rule edits. One line, exactly where it belongs —
BiblePage.razor:290, nestled betweenProjectIdandcharacters, matching every sibling table. I traced theSyncRowslive set top to bottom: glossary, ProjectId, houseRules ✓, characters, lore, beats, summaries — complete now. The edit can no longer be classified as a ghost.The regression test (
A_pending_house_rule_edit_survives_a_reload_from_another_section) is the real prize. It doesn't just assert "the debounce works" in isolation — it races the two the way production does: type into a rule, then trigger a structural write in a completely different section (lore add) so the reload lands mid-debounce. That's the exact failure mode I flagged, andWaitForAssertiononrule.RulewithSaveWindowproves the edit flushed to the store rather than vanishing. You verified it fails without the fix — that's how you earn a test's keep~ ♪💡1 —
NormalizevsText.BlankToNull. Byte-for-byte identical semantics (I traced both: trim-then-IsNullOrEmptyvsIsNullOrWhiteSpace-then-trim arrive at the same result for every input class), and the private method is gone. The domain tests pin blank-origin-becomes-null unchanged — behaviour never moved, only its address. One copy, as it should be.💡2 — the delete label.
$"house rule "{Excerpt(row.Rule)}""— now it names what it's killing, like every sibling does.Excerptat 60 chars on a word boundary with…is the right shape: prose isn't a title, so you can't interpolate it raw, but you need enough to tell two rules apart. Clean.✅ What I liked~
Excerptisprivate static— a pure helper, no state, no side effects. Exactly right.839/839 green (+1, the regression test), build 0 warnings/0 errors. Happy~ ♡
Automated review by Jibril · 2026-07-28
CI/CD: stale for
9ef856d(coverage bot 4892 coversd7377adonly) · Local checks: build 0/0, 839/839 pass