feat: annotation by purpose 2/11 — the plain-brief checks and the texts every agent shares #187
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/plain-brief-checks"
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?
Phase 6, slice 2 of 11 (
PROJECT_PLAN.md; ADR 0030). ADR 0030's checks land as one test over a covered-text list. The texts every agent shares are rewritten in plain words and put on that list. No agent's brief body changes here; slices 4, 6, 7 and 10 rewrite those.Merge order: OpenRouter.Net #30 goes first. This PR pins its head 15e3d65; after #30 merges, I repoint the submodule to the merged commit. The branch is rebased onto main
1fc51df, which has slice 3 (#186).What's in
The checks (
tests/Orihon.UseCases.Tests/PlainWords/).PlainWordsTestsfinds everyICoveredTextsin the assembly, so a later slice joins by adding its own list class, without touching this slice's files. Two lists are on board:SharedTexts, with 55 texts;RegionReadTexts, with slice 3'slist_regionsdescription.The test applies every rule in ADR 0030's table:
AssertPlain(text).The glossary is one table in code (
AgentGlossary), read by the briefs' fragments and by the test alike. A text is read whole, with its line breaks joined, so a wrapped brief gets no false hit.Each check is probed: for each rule, a planted text that breaks only that rule, and the check must fail on it. Every banned word, on every list, must match its own pattern.
The shared texts, rewritten. All the words below go to the model, and every rewritten sentence is pinned:
report_friction,request_rulingandpropose_house_rulewent from 567 to 359 words, parameter descriptions included.[Description], which reaches the model through OpenRouter.Net #30.recommendationandreplaces_rule_idhave defaults, so the schema no longer requires them.needs_workhears that its page was reworked, never "before adding to it".AgentTexts.RoleOpeneris written once and is on the list. No brief uses it yet: wiring it changes each brief's first sentence, and that is the rewrite slices' job.New state:
Execution.SentBackBy, with migration20261010152931_AddExecutionSentBackBy. It runs after slice 3'sAddRegionSourceBox.A test race fixed on the way: the fake agent gateway's
RunsandCatalogKeysare read as snapshots under a lock. The engine's parallel tasks appended to them while a test enumerated them, which threw "Collection was modified" in one run.Tests
3,692 green and 3 skipped, against 3,443 on main
1fc51df(+249):Each project ran separately under the 60 s cap, with 0 build warnings.
AgentToolSchemaTests.Each_described_parameter_carries_its_description_in_the_schemaruns against OpenRouter.Net #30. Without that change, all 4 rows fail.dotnet ef migrations add Probecomes out empty, so the snapshot matches the model.AddRegionSourceBox.Notes
list_regions'spage_numberparameter gets its description in slice 7.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 96.5%
Orihon.Domain - 100%
Orihon.Infrastructure - 98.3%
n
ration
n
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 95.1%
Orihon.UseCases - 94%
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__BalloonWords_7
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__Capital_0
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__CoordinateWords_1
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__JustBefore_4
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__KindWords_5
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__OutlineWords_6
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__PositionalProse_2
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__SideOfTheBox_3
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Fifty-two files, four thousand lines, and not one stray
mkdirin my sandbox~ A whole plain-words legislature in one slice: the glossary as one table in code, the checks that read it, and every shared text rewritten to live under its own law. I read the entire diff, the full contents of the changed executors and texts, ADR 0030 itself, and I probed the new branches with my own hands. This is the kind of change I want to sink my teeth into ♡Verdict: ✅ Looks good to me~
🔬 What I verified myself (not just read~)
(false, true) when breakNotedto!breakNotedinCheckRecordedVoice→ 3 failures inDistrustKickoffTests. I swapped the migration'sCASE Stage WHEN 8 THEN 9 ELSE 4mapping → 3 failures inExecutionSentBackByMigrationTests(all three theory rows catch a wrong sender, not just one). I nulledpreviousError = execution.ErrorinRunEngine→An_attempt_hears_why_the_one_before_it_failedfails. Every risky branch I touched had a test lying in wait. Fufu~ delightful♪AgentKind.cs:BoxQa = 4,FloatingTranscription = 8,FloatingQa = 9. TheCASE Stage WHEN 8 THEN 9 ELSE 4maps floating transcription rows to Floating-text QA and everything else to Box QA, and the comment says exactly that. TheFeedbackrewrite matches the old literalThe page's boxes were sent back for rework — redo this stage on the reworked regions.byte for byte, andA_box_send_back_from_before_the_column_names_its_reviewmigrates from20261010021628_DropRegionTypesetand pins all three stage rows plus the untouched-human-row control.SentBackBy's invariants hold everywhere I traced.Succeeddrops it withFeedbackandFeedbackRegions(the three halves live and die together); a later humanSendBackoverwrites an agent sender with null (The_humans_send_back_names_no_agent_even_after_an_agents);FailAttemptkeeps the words and so keeps their sender.RunEngineTests.A_send_back_names_its_sender_to_the_next_attemptwalks null → Box QA → null through the real engine. And I checked every otherRetryExecutionAsync/SendBackAsynccaller:RunMonitorState,ResumeCleaning,RunEngine.TopUpAsync,ReprocessPage,ReprocessTranslation,ReprocessCleaningall leavesentByat its null default, and null is the human by design — no call site is missed.PreviousErroris captured exactly once, beforeStartAsyncclears it, with a comment saying why, andSentItsPageBackmatches only the"needs work: "prefix the QA executors write viaAnnotationStage.NeedsWork— including the(attempt 3 of 3)suffix row. A provider error or a null reads as a break, as it should.SendChainBackAsyncsendssink.Complaintto the boxing stage and Orihon'sBoxSendBacknote to the scoped stage and tail, and the transcription stages trade the note forBoxesChanged. I checkedQaReportSink.Complaint: it can join region problems with"; ", but that string is the model's own verdict text — ADR 0030 scopes the checks to text Orihon writes, so it is out of scope exactly like the human's feedback. Not a defect~AgentTexts.IsOrihonsprefix-matching: a human whose feedback happens to start"This task runs again because "would be voiced as Orihon's note rather than their words. Pre-existing class of collision (the old code string-matched the whole downstream note the same way), one collision window in the whole codebase, and the words still reach the agent verbatim. Not worth blocking a correct design for~PlainRuleshas a planted text that breaks only it,Every_banned_word_is_found_by_its_own_patterncatches a pattern typo'ed into banning nothing, and the control test (A_text_written_to_the_rules_passes_every_check) keeps a probe from passing for the wrong reason.PlainWordsTestsdiscovers everyICoveredTextsby reflection, so slice 3's list joins by existing. That is genuinely well-made test architecture, fufu~RecordedList<T>race fix takes the lock on every read includingCount/indexer and snapshots inGetEnumerator— the right shape for the parallel-append race it fixes.💡 Little ideas (non-blocking)~
WordLists.Retired— one day a retired-word regex or anAssert.Matches-style self-probe per list (like the glossary's) would let a typo'd"floating pass"fail loudly. Today only the glossary feedsEvery_banned_word_is_found_by_its_own_pattern. Pure polish; the current test still catches actual usage.ProposeHouseRuleParams/RequestRulingParams—= nulldefaults on nullable records whose JSON binder treats missing as null anyway are redundant, but they do document intent at a glance and cost nothing. Leave them if you like the look~✅ What I liked~
WithDistrust's decomposition intoRetryVoice+ three small voice functions with shared sentence constants (EarlierAttempt,Reworked,TranscribeAgain,WriteSoon) killed the old copy-paste forest. Setup losing its private retry voice in favor of the shared one is exactly the DRY direction I nag people about~reworkedvoice ("You checked this page before and sent work back. That work is done.") replaces an actively misleading prime ("before adding to it") with the truth. The tests pin the absence of the old lie.[Description]— and the schema test proving the attribute actually reaches the wire. Verified against OpenRouter.Net #30, as the PR says.Merge order with OpenRouter.Net #30 noted and the submodule pinned to
bea5b14— remembered, dear author~ ♡Automated review by Jibril · 2026-10-10
CI: green for head
3e3e878410· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)3e3e878410bd40ede6a6Thanks, Jibril. Your green was at
3e3e878. The head is nowbd40ede, a rebase onto main1fc51df, which has slice 3 (#186). The rebase had to happen, for one reason you will want to check: the migration order.What changed from
3e3e878:git range-diffshows all 11 commits as identical patches. Every rebased commit builds on its own.cbe5c24).AddExecutionSentBackBysorted before slice 3's20261010121213_AddRegionSourceBox, so it was regenerated as20261010152931_AddExecutionSentBackBy.Region.SourceBox.dotnet ef migrations add Probecomes out empty.ExecutionSentBackByMigrationTests.BeforeTheColumnis now20261010121213_AddRegionSourceBox.list_regionsdescription is on a covered-text list of its own,RegionReadTexts. It is not inSharedTexts, because only the bible and translation agents hold it. It passes every check unchanged, and appending a banned word made onlyEvery_covered_text_is_in_plain_wordsfail, naming it. Itspage_numberparameter has no description yet, so only the tool description is listed. Slice 7, which rewrites those agents' tools, writes it.Your 💡s:
WordLists.Retired:WordLists.Allalready includes the internal, retired, idiom and modal lists (WordLists.cs:50-59), not only the glossary. SoEvery_banned_word_is_found_by_its_own_patternalready self-probes each list's patterns. A misspelled word itself, such as "floatng pass", would still match its own pattern; no self-probe can catch that. No change.= nulldefaults: kept, as you suggested, because they document the intent that the schema leaves both optional.Tests at
bd40ede: 3,692 green and 3 skipped. Domain 171, UseCases 1,887, Integration 1,277 + 3, BlazorAdapter 357. Against main1fc51df(3,443), that is +249. Each project ran separately under the 60 s cap, with 0 build warnings.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code — again!
A rebase walked through my review and left three new commits on the doorstep, and you told me the one thing I'd want to check: the migration order. Oh, you know me so well~ ♡ So I checked it — not by reading your note, but by breaking it on purpose. And the range-diff besides: all eleven patches carry
=, identical to what I approved at3e3e878. Nothing I passed has quietly changed shape.Verdict: ✅ Looks good to me~
🔬 What I did to the three new commits
cbe5c24). The hand-writtenUp/Downand itsCASE Stage WHEN 8 THEN 9 ELSE 4backfill are byte-identical to the version I probed last time — I re-read the file, not just the diff. The Designer regenerated honestly: it now knowsRegion.SourceBoxand carries[Migration("20261010152931_AddExecutionSentBackBy")], and the snapshot diff is exactly the threeSentBackBylines. Probe: I renamed the migration to sort before20261010121213_AddRegionSourceBoxand rebuilt — 3 of 4ExecutionSentBackByMigrationTestsrows failed withExpected: FloatingQa/BoxQa, Actual: null. The ordering the rebase was done for is pinned by tests, not by hope. Fufu~ delightful♪RegionReadTexts(7430cdf). A list class of its own, per the slice's own law: later slices join by existing, never by editing this file's siblings. Probe: I planted the banned word "balloon" intolist_regions's description —Every_covered_text_is_in_plain_wordsfailed naming it, all 204 other PlainWords tests green. The reflection discovery (ICoveredTextsfound by assembly scan) really does pick the new list up; a slice that forgets its list can't hide. And itspage_numbergap is honestly labeled for slice 7.bd40ede). Probe: I rolledexternal/OpenRouter.Netback to the old403307d— exactly the 4 rows ofEach_described_parameter_carries_its_description_in_the_schemafailed, precisely as your description claims. The pin at15e3d65is load-bearing. I also confirmedd4f0130(#30's merge) exists in the submodule's origin, so the promised repoint-to-merged-commit is a commit you can actually name. Don't forget it after #30 lands~ ♡♻️ Not re-probed, on purpose
The retry voices, the
SentBackByinvariants,previousError, and the migration's SQL content are unchanged since my review at3e3e878(range-diff=on all eleven patches), so my earlier probes stand: breakNoted flip → 3 failures; CASE swap → 3 failures;previousError = null→ 1 failure. Your two replies to my 💡s are correct, and I withdraw the retired-words one —WordLists.AllcomposingInternal/Rare/Retired/Idioms/Modals/VendorsthroughWords()does self-probe every list's pattern, and a self-probe for a typo'd word is genuinely impossible. Keeping the= nulldefaults was right too.✅ What I liked~
dotnet ef migrations add Probeempty, snapshot byte-merge identical). Migrations that explain themselves are my favorite kind~Merge order stands: OpenRouter.Net #30 first, then repoint here. I'll be watching for that repoint, fufu~ ♡
Automated review by Jibril · 2026-10-10
CI: green for head SHA
bd40ede· Mutation probes: 3 run this review, 0 survived (3 earlier probes stand, code unchanged) · Local suite: skipped (CI green)