Split region notes into stage-owned fields #203
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "stage-notes"
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?
The one region
Notesfield becomes three, one per stage (ADR 0012 amended):add_region's required note), Box refinement adds to it (note_regionrenamednote_box, its own description naming its field)reject_region'srejected: …marker lands here, a landing source strips it,set_transcription's old note surgery is gonenote_translation(page-number addressed, append semantics) in its grant and briefForward visibility, read-only in reverse — each stage sees the earlier stages' notes (briefings say "Its box note says …",
list_regionslabels each by stage; the bible agent's reads show all three) and no stage holds a tool that writes an earlier stage's note. That invariant is enforced by the grants and pinned by test (A_stage_note_write_leaves_the_other_stages_notes_untouched, mutation-verified). The transcriber keeps its bias guard: its own note never reaches its briefing.Migration splits the legacy column by the rejection marker: everything before
rejected:stays the box job's, the marker onward becomes the transcription's;Downrejoins them.Brief wording follows ADR 0030's plain-words regime (job-named "Transcription note" — the bare noun is a banned source-synonym); every moved sentence carries its pin, new sentences are pinned, and the seed exercises all three notes.
Verified: Domain 185/185, UseCases 3186/3186, Integration 1396 passed (3 skipped, pre-existing), BlazorAdapter 409/409.
Summary
Summary
Coverage
Orihon.BlazorAdapter - 96.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 98.4%
n
ration
n
one
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 95.2%
Orihon.UseCases - 94.6%
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__BalloonWords_7
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__Capital_0
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__CoordinateWords_1
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__JustBefore_4
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__KindWords_5
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__OutlineWords_6
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__PositionalProse_2
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__SideOfTheBox_3
- The migration splits the legacy note segment by segment, as the tools joined it with " — ": a `rejected: ` segment goes to the Transcription note, every other one stays the box job's. The marker split left a dangling " —" on the box note and moved a box sentence written after a rejection into the Transcription note. Down rejoins all three notes. Pinned by RegionStageNotesMigrationTests (upgrade and downgrade). - note_translation moves beside set_translation; the page-and-label lookup it, set_translation and get_region each copied becomes PageByNumber.FindRegionAsync. Its append, its refusals and the notes it leaves alone are now tested. - The stage-note pin is a theory over every region write of the box and reading grants, and the move-resize threading test asserts all three notes. - The glossary's "note" covers all three notes; tool outputs and briefs name them alike ("box note", "Transcription note", "Translation note"); Transcription QA reads the box note again; cleaning's list_regions shows the Transcription note again. - Box refinement's brief names Box creation and floating marks again, within the length cap. - `notes` parameters become `boxNote`; a stale rejection comment is corrected; ADRs 0012, 0016 and 0017 list note_translation and the segment split. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>Follow-up from a review pass, in
263093don top offe38121.Fixes
rejected:marker left a dangling—on the box note ("the bubble top right — rejected: …"→ BoxNote"the bubble top right —"). It also moved any box sentence written after a rejection into the Transcription note. The split now goes segment by segment, the wayWithoutRejectionsreads the old field:rejected:segments go to the Transcription note, every other segment stays in the box note, each in order.Downrejoins all three notes instead of dropping the Translation note when a box note exists. Pinned by the newRegionStageNotesMigrationTests: an upgrade fromAddRegionBoxApprovedover five note shapes, and a downgrade.note_translationhad no behaviour test (coverage bot: 27.2% line / 0% branch). It moves fromAnnotation/RegionAuthoringTools.cstoTranslation/TranslationTools.cs, besideset_translation, which drops the strayusing …ResearchSetup. The page-and-label lookup that it,set_translationandget_regioneach copied is nowPageByNumber.FindRegionAsync. New tests:Note_translation_adds_to_the_Translation_note_alone(append, same sentence twice is a no-op, label case ignored, box and Transcription notes, source, target and needs-translation untouched) andNote_translation_refuses_a_blank_note_and_a_region_the_page_lacks.A_region_write_leaves_the_notes_of_the_other_stages_untouchedis now a theory over every region write in the box and reading grants: bothmove_resize_regions,classify_region,note_box,set_transcriptionandreject_region.Move_resize_threads_every_other_profile_field_throughnow also asserts the two notes it seeds.Wording
notestill defined only the box note, yet the translator's brief now lists it. It now covers all three notes, without "rejection", which the Box refinement brief must never say.box note,Transcription note,Translation note(the job-named form, as in the briefs).list_regionsandget_regionwere printing lowercasetranscription note:/translation note:.list_regionsshows the Transcription note again. Before the split, cleaning saw the rejection reason inside the one note.For a \bubble`` rewrites kept their meaning and stay.notesparameter of theRegionconstructor and ofCreateRegionis nowboxNote;add_region, its refusal andnote_boxsay "box note"; the stale "a later reading replaces the whole note" comment is corrected; the test fixtures put rejections in the Transcription note.note_translation.Tests: Domain 185/185, UseCases 3193/3193 (+7), Integration 1398 passed with 3 skipped (+2), BlazorAdapter 409/409.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Stage-owned notes~ One field becomes three, each stage keeping its own diary, and the migration even does segment surgery on the legacy column. I read the whole split twice — the recursive CTE genuinely made me giddy. But fufu… one little branch thought it could hide from the tests. It can't. Not from me~ ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
list_regionshas no test, and I proved it the sharp way: I deleted the entire line, rebuilt, and ran every Cleaning test — 897/897 still passed. A surviving mutant~ ♡ This PR deliberately re-gives the cleaning survey the rejection reason ("Before the split, cleaning saw the rejection reason inside the one note"), yet nothing pins it: the pinned output inCleaningToolTests.List_regions_says_what_cleaning_does_with_each_region(tests/Orihon.UseCases.Tests/Cleaning/CleaningToolTests.cs:311) seeds only a BoxNote, so the branch is never asserted. One day somebody "simplifies" that line away, the survey silently loses every rejection, and no test will scream. You wouldn't leave THAT in production, would you? ♡Fix: extend that test (or add one) with a region seeded
TranscriptionNote = "rejected: …"and assert theTranscription note: …segment appears in the listed output.The same display in the annotation grants'
list_regions(src/Orihon.UseCases/Agents/Annotation/RegionAuthoringTools.cs:723) is pinned only negatively (Assert.DoesNotContain("rejected:"), AnnotationToolTests.cs:1305). I checked every expectedTranscription notestring in the suite — only the bible'slist_regionshas a positive pin (BibleRunTests.cs:165). Give this one the same one-line pin while you're there, please~💡 Little ideas (non-blocking)~
✅ What I liked~
WithoutRejectionsread the old field — and pinned by an upgrade test over five note shapes (including the interleavedthe sign — rejected: too faint — kept the sign…and a double rejection) plus a downgrade. I mutated therejected:routing away and the test killed it on the spot. Delicious~ProfileOf(TranslationNote → null) and all seven tests failed at once. THAT is an invariant with teeth.NoteRegionTool.Appendfornote_box,reject_regionandnote_translation— same append, same retry no-op, zero copy-paste. AndPageByNumber.FindRegionAsyncgives the three copies of page-and-label lookup one home at last.Automated review by Jibril · 2026-10-16
CI: green for head SHA
263093db42· Mutation probes: 3 run, 1 survived (cleaninglist_regionsTranscription-note line deleted → 897/897 Cleaning tests still green) · Local suite: skipped (CI green)Round 1 addressed in
16c56db.list_regionsTranscription-note line.CleaningToolTests.List_regions_says_what_cleaning_does_with_each_regionnow gives p1r4TranscriptionNote = "rejected: the box holds no mark"beside its box note, and the pinned output ends…, box note: the small mark, Transcription note: rejected: the box holds no mark. The annotation grants' line has its positive pin as well:A_rejection_adds_its_reason_to_the_reading_note_and_keeps_the_box_notecalls Transcription QA'slist_regionsafter thereject_regionand asserts, box note: the jagged text over the device, Transcription note: rejected: illegible as close as the views go. I re-ran your probe on both: deleting the cleaning line failsList_regions_says_what_cleaning_does_with_each_region, and deleting the annotation line fails the rejection test.SeedDevDataTestsasserts all three on that region. I seeded a note a human wrote on the reading (the trailing dots are drawn, not typed) rather than a rejection. A rejection would empty that region's source and stop it needing translation, which shifts the page states the seed documents in AGENTS.md. The reading's channel shows either way, and the rejection display is pinned by the two tests above.Domain 185/185, UseCases 3193/3193, Integration 1398 passed with 3 skipped, BlazorAdapter 409/409. The counts are unchanged: both pins extend existing tests.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Back so soon~? Fufu, you came, you pinned, you conquered. Round 1 I left you one surviving mutant and a wish about the seed — and this push hands me both, gift-wrapped. Naturally I didn't take your word for it. I never do~ ♡
Verdict: ✅ Looks good to me~
✅ What I liked~
Transcription notesegment out of cleaning'slist_regions(CleaningTools.cs), rebuilt, and ran the pinned test —List_regions_says_what_cleaning_does_with_each_regionfailed on the spot (1 failed / 1 total). The mutant that survived round 1 is dead.Assert.Equalover the whole pinned line, new segment and all — the strongest kind of pin. Delicious~Transcription notesegment fromRegionAuthoringTools.cs'list_regions, rebuilt —A_rejection_adds_its_reason_to_the_reading_note_and_keeps_the_box_notefailed (1 failed / 1 total). The review now reads the reason where the reader left it, proven end-to-end through Transcription QA's own grant. That's the sibling parity I asked for~SeedDevDataTestsasserts all three on that one region — tighter than before, where two separateAssert.Containscould have matched two different regions. And your choice of a human-written note (the trailing dots are drawn, not typed) over a rejection is exactly right: a rejection would empty the source, drop the region out of needing translation, and shift every page state the seed documents in AGENTS.md. The rejection display is pinned by the two tests above; the seed doesn't need to duplicate it. Correct reasoning, correctly executed.22 lines — assertions and one seeded property, zero production changes. So my round-1 probes on the migration CTE and thean unbroken collection ♡ProfileOfnote threading stay valid (code and tests untouched; both failed loudly at263093d). Every mutant I have ever planted on this PR is now dead. Not one survives. …FufuCI green (build + test) for
16c56db2cd; local full suite skipped per the CI-green rule — my two probes above were the only local runs.Automated review by Jibril · 2026-10-16
CI: green for head SHA
16c56db2cd· Mutation probes: 2 run, 0 survived (2 skipped — probed code unchanged since263093d, both killed there) · Local suite: skipped (CI green)