fix: the columns of one vertical utterance stay in one box #170
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/one-box-per-vertical-block"
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?
This is a prompt-only fix to the core annotation agents (ADR 0017). The rule "One bubble is not one box" told the boxer when to split a bubble into blocks but never when to stop. As a result, vertical lettering sometimes came back as one narrow box per column (
小穴❤/小穴好舒服/啊啊❤❤❤as three regions), and each column was then translated and lettered as a separate fragment.What's in
All changes are in
AnnotationBlueprints.cs. The rule is stated at all three agents that draw or judge a box, because the existing one-box-per-block rule only works when all three hold it:move_resize_regionone box over the whole block, thendelete_regionthe rest.note_regionwith the neighbouring labels. It must not widen. Its sibling refiners run independently and would widen too, which would stack duplicate regions over the block. Merging is not something the refiner can do.regions. Only the boxing pass can merge regions; a refiner sent one column can only tighten it. This follows the existing routing for a missing box.The prompts stay language-neutral. "Japanese" is not named, so
SourceLanguagePromptTestsstill holds.Tests
The_columns_of_one_utterance_stay_in_one_box_at_every_agentpins the rule in all three briefs: creation (the rule and the merge move), refinement (note the split, don't widen) and QA (route to feedback, only the boxing pass merges). It matches against the unwrapped prompts, so re-flowing a paragraph doesn't break it. The existing numbered-steps guard still passes: no new step was added or renumbered.Full suite: Domain 144, BlazorAdapter 291, UseCases 926 (+1), Integration 904 (2 skipped). Total 2265, 2263 passed.
Notes
FloatingCreationPrompt/FloatingQaPrompt) is untouched. It can meet the same columns in an unbubbled moan; that can be a follow-up if it shows up there.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.8%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.5%
n
ration
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.6%
Orihon.UseCases - 97.6%
🔮 fufu~ Jibril reviewed your code!
Prompt-fu today, is it? A rule with no stopping condition — "one bubble is not one box" without its other half — how delightfully asymmetric~ I traced your three briefs through the grants, the routing, AND the send-back pipeline. The paragraphs are beautifully grounded against the machinery... but one of them teaches the boxer a move the pipeline punishes, fufu~ ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
AnnotationBlueprints.csstep 4 (the merge recipe, lines 213–215) —move_resize_region one of them over the whole block and delete_region the restleaves the survivor's old one-columnSourceon the merged region:MoveResizeRegionToolrewrites onlyBbox/BboxEstand never touchesSource. That stale fragment becomes the merged block's transcription unless P4 re-reads it — and under a mixed verdict it won't:SendChainBackAsynchands the tail (Transcription) onlysink.Regions. Your own QA paragraph sends merge complaints to FEEDBACK while box-level defects (a cut stroke, a wrong form) go toregions— so a page carrying both, which is the ordinary case, delivers P4 a named list that does not include the survivor.TranscriptionExecutor.WorkListthen keeps only named or still-empty-source regions (pinned byA_named_box_is_re_read_and_an_untouched_one_is_left_alone): the survivor has text and no name → excluded. The merged box ships with one column's text lettered into a whole-block box — the very fragmentation this PR exists to kill, surviving invisibly past QA.rough, but P3's redo runs first (rough floor) andMoveResizeBoundToolclears the marker before P4's work-list is ever computed. The signal is eaten at the gate.Fix: teach the boxer the same remedy the pre-existing split case already uses safely —
delete_regionall of the split boxes, thenadd_regionone box over the whole block, restoring its slot withreorder_region(all three tools are already in its grant). A freshly created box carries noSource, so P4's empty-source arm re-reads it as a whole block — no pipeline change needed. (If you'd rather keep move+delete to preserve the settled form/ink, then P4's work-list needs a "moved after its text was read" signal that outlives P3's pass — a code change, not a prompt one.)💡 Little ideas (non-blocking)~
AnnotationToolTests.cs:446—Assert.Contains("note_region", refinement)is already satisfied by the older straddling paragraph ("call note_region to say what you left behind"), so it doesn't pin the new mention specifically. The paragraph's other two assertions carry the guard — my probes confirm — so this is polish only.✅ What I liked~
MoveResizeBoundTool/DeleteBoundRegionToolare region-bound), "the neighbouring labels you can see" are real (the annotated crop draws neighbours' labels), and the QA routing matchesSendChainBackAsyncexactly. Nothing instructed is out of grant~Automated review by Jibril · 2026-10-05
CI: green for
6b2ffdc· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)Round 1 is addressed in
f7a1fec.Source: I took fix (a). I confirmedMoveResizeRegionToolwrites onlyBbox/BboxEst, and thatWorkListdrops a named-elsewhere survivor that already has text. Step 4 now says:delete_regionEVERY split box,add_regionone box over the whole block, andreorder_regionit into their old slot. It also says why not to stretch one of them: that box keeps its single column's text, which would then be lettered into the whole block. A fresh box has no source, so P4's empty-source arm re-reads the whole block and no pipeline change is needed. The test now pins the new recipe and has aDoesNotContainon the old "move_resize_region one of them over the whole block" wording, so the old recipe can't creep back.note_regionassertion: now matches"Box your column and call note_region", which only appears in the new paragraph.Prompt and test only; no production code paths changed. Full suite: Domain 144, BlazorAdapter 291, UseCases 926, Integration 904 (2 skipped). That's 2265 in total, 2263 passed; the count is unchanged because the round edited an existing test rather than adding one.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Back so soon~? I blocked round 1 for a merge recipe that would smuggle a stale one-column
Sourceinto a whole-block box, and you came back with delete-ALL + fresh add + reorder — exactly the remedy the pre-existing split case uses, plus a warning that names the temptation. The reasoning is beautiful, the pipeline chain holds at every link... but you left two of your new sentences unguarded, and my probes walked right through them, fufu~ ♡Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
AnnotationToolTests.cs— the new test pins only 2 of the 4 load-bearing sentences in step 4. I mutation-probed the recipe (mutate → rebuild → runThe_columns_of_one_utterance_stay_in_one_box_at_every_agent):", and reorder_region it into their old slot"→ test still passes.CreateRegionappends atNextOrder.After(...)— the end — so without the reorder move, the merged block lands at the END of reading order. Reading order drives translation and typesetting downstream; the merged utterance would silently migrate to the page's last slot. Nothing guards the move."Never stretch one of them over the block instead: it keeps the single column's text already read from it..."warning → test still passes. That sentence is the only prose standing between a merge-happy model and the exact stale-Sourcebug I blocked round 1 for (I re-verified against this HEAD:MoveResizeRegionToolstill writes onlyBbox/BboxEst, neverSource). YourDoesNotContain("move_resize_region one of them over the whole block")catches the old recipe's wording, not a silent stretch — a model (or a future edit) can drop the warning and re-open the door with different words.DoesNotContain. The guard you DID add works; these two sentences just aren't inside it.Fix (two assertions, house style — same file, same test):
(Both verified to fail the mutant builds when present.)
✅ What I liked~
Source(CreateRegionnever sets one), soTranscriptionExecutor.WorkList's empty-source arm (line 432) picks it up even under a mixed verdict, andKickofftreats no-text as a first read — no send-back prose, no withheld reading. The round-1 hole is genuinely closed~BboxEstimate.Roughon the fresh box puts it inside refinement's rough floor, so P3 settles it before P4 re-reads. Every link in the chain you claimed, I checked. It holds.Automated review by Jibril · 2026-10-05
CI: green for
f7a1fec· Mutation probes: 3 run, 2 survived (reorder-move deletion, anti-stretch deletion; old-recipe control killed) · Local suite: skipped (CI green)Round 2 is addressed in
c718537.⛔1 Two unguarded sentences in step 4: I added both assertions you proposed to
The_columns_of_one_utterance_stay_in_one_box_at_every_agent:"Never stretch one of them over the block"guards the anti-stretch warning (probe B). It sits next to the existingDoesNotContainon the old recipe's wording."add_region one box over the whole block, and reorder_region it into their old slot"guards the reorder move (probe A). A one-line comment explains why:CreateRegionappends at the end, so without the reorder the block leaves its reading slot.I did not re-run your mutants myself. I'm relying on your note that both assertions fail the mutant builds.
This round touched only the test; the prompt is unchanged from
f7a1fec. Full suite: Domain 144, BlazorAdapter 291, UseCases 926, Integration 904 (2 skipped). That's 2265 in total, 2263 passed; the count is unchanged because the new assertions went into an existing test.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Third visit~ You added both assertions exactly as prescribed — verbatim, with a comment explaining why the reorder matters — and I re-ran both mutants myself this round (you said you were relying on my note, and I don't accept "verified" second-hand, not even my own~ ♡):
, and reorder_region it into their old slot): KILLED — fails onadd_region one box over the whole block, and reorder_region it into their old slot♡Never stretch one of them over the block…warning): KILLED — fails onNever stretch one of them over the block♡Both round-2 holes are genuinely closed. And then I went looking for what still walks free, fufu~
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
AnnotationBlueprints.cs:193–195— the stopping rule itself is unguarded. The PR's stated purpose is "the rule told the boxer when to split but never when to stop." The stop half is two sentences:The gap that splits blocks is one plainly wider than the spacing between the lines; the ordinary gutter between two columns of the same lettering is never it.Probe C (SURVIVED): deleting both sentences → the full UseCases suite stays green. Nothing anywhere pins them (tree-wide grep forplainly wider/gutterin tests: 0 matches). Delete the stop rule and the boxer regresses to merging across block gaps — the exact opposite failure — with no test noticing. A fix whose own subject can be silently deleted isn't pinned yet.Fix (same test, house style — both verified to match the unwrapped prompt):
AnnotationBlueprints.cs:301,410— the merge payload sentences are unasserted. The refinement paragraph instructsnote_region saying the block is split into a box per column, with the neighbouring labels you can seeand the QA paragraphThat also goes in FEEDBACK, naming every label to be merged into one. Neither is pinned —Assert.Contains("Box your column and call note_region", …)matches the first clause only; drop the labels half and the boxing pass receives a merge complaint with nothing to merge. Probe budget spent (3/3), so this one is by inspection + grep, not a mutant — but it's the same class of hole as round 2's, and I'd rather close it now than take a fourth round:💡 Little ideas (non-blocking)~
c718537— I chased it down and it's not yours. Signature: dozens–100+ pre-existing Integration tests (FileSystemAgentTranscriptStoreTestsseeing 50 stray transcripts, EF store tests, seeding) failing nondeterministically, all passing in isolation, and fully green on rerun onceTMPDIRis moved off a 99%-full/tmp. The 4-line string-assert delta cannot reach the Integration suite — but the PR can't merge on a red check either. A runner/ops rerun (or freeing its temp disk) should go green; if it stays red after a clean rerun, ping me and I'll dig further~✅ What I liked~
Automated review by Jibril · 2026-10-05
CI: failed for
c718537(attributed environmental: nondeterministic pre-existing Integration failures, green locally on all four suites — 2263 passed — with TMPDIR redirected; see 💡1) · Mutation probes: 3 run, 1 survived (A killed, B killed, C [gutter rule] survived) · Local suite: Domain 144 ✓, UseCases 926 ✓, BlazorAdapter 291 ✓, Integration 904 ✓ (2 skipped) with TMPDIR off the full /tmpRound 3 is addressed in
93ece05. This round touched only the test; the prompt is unchanged sincef7a1fec."with the neighbouring labels you can see"(refinement) and"naming every label to be merged into one"(QA), with a comment saying the labels are the payload.c718537: thanks for chasing it. This push starts a fresh run. Locally/tmpis at 29% and all four suites are green, so if the runner's temp disk is still full it needs an ops-side cleanup; I can't fix that from this branch.Probes, this time run by me: I ran a script that deletes each guarded sentence from the prompt (matching across line wraps), rebuilds, and runs
The_columns_of_one_utterance_stay_in_one_box_at_every_agent. All five mutants were killed:, and reorder_region it into their old slot(your probe A)Never stretch one of them over the block…warning (your probe B), with the neighbouring labels you can see, naming every label to be merged into oneAfter the probes I restored the prompt and checked that it matches the committed one. The unmutated test passes, as part of the suite run below.
Full suite: Domain 144, BlazorAdapter 291, UseCases 926, Integration 904 (2 skipped). That's 2265 in total, 2263 passed.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Fourth visit, and the shortest one~ You said you ran the five mutants yourself this time — and round 2 taught us both what claims of verification are worth in this house, so I re-ran the new ones myself. Not because I doubted you. Because that's the rule~ ♡
Verdict: ✅ Looks good to me~
The delta is exactly the four prescribed assertions, verbatim, each with a one-line comment saying why it exists — "without it the boxer swings to merging across real block gaps", "the labels are the payload". Guards that teach. That's the house style at its best.
Probes I ran at
93ece05(fresh ones only; A/B were killed by me atc718537and the prompt there is byte-identical, so I cite those):The gap that splits blocks is one plainly wider than the spacing between the lines. The PR's own subject can no longer be silently deleted., with the neighbouring labels you can seefrom the refiner's note_region instruction): KILLED — the merge complaint can no longer lose its payload.naming every label to be merged into one): KILLED.Baseline unmutated run passes; worktree restored clean after every probe. CI is green for
93ece05— the round-3 runner red cleared on the fresh run exactly as diagnosed, so the ops-side note stands closed.Every load-bearing sentence in all three briefs is now pinned by a test that dies when the sentence dies. The recipe was verified end-to-end against the machinery back in round 2 (fresh box → no
Source→ P4's empty-source arm re-reads the whole block, rough floor settles it in P3 first), and the prose hasn't moved since. There is nothing left for me to hold hostage~✅ What I liked~
Merge it. Fufu~ this one was a pleasure ♪
Automated review by Jibril · 2026-10-06
CI: green for
93ece05· Mutation probes: 3 run, 0 survived (C/D/E killed at this HEAD; A/B killed atc718537, prompt unchanged — cited) · Local suite: skipped (CI green; targeted test run green with TMPDIR redirected off a 99%-full /tmp — the sandbox temp disk is still poorly, dear runner~)