feat: create a chapter with its pages already in it #187
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/chapter-creation-batch"
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?
Closes #186.
What's in
doujin_write'screate_chaptermade one empty chapter, and filling itwas a separate
move_pages_to_chaptercall. Splitting a book into fourchapters therefore cost eight approval cards, and the model could not even
shorten that by firing the creates together: tool calls in a round run in
parallel (
Future.waitin the vendored agent), so twocreate_chaptercallsread the same chapter list and hand the server the same sort order — the tie
the append was written to avoid.
chaptersarray oncreate_chapter— one entry per chapter, each witha
titleand thepage_numbers(orimage_ids) that belong in it. Thewhole split is ONE call on ONE card: N creates, the page moves, and the one
reorder that puts the block at a
position. The single-chapter form keepsworking and takes a page selection too; an entry with no pages still creates
an empty chapter.
pages come first in reading order, so the moment one chapter is filled the
numbers the model holds for the next one mean different pages — filling
"pages 4-6" first makes "pages 1-3" name exactly those same pages again. All
entries resolve against the variant the model read, and the writes go by
page id.
get_doujinprinted, any chapter those pages are taken out of is namedseparately, and a chapter the split leaves behind with nothing in it says
so. A page named by two entries is refused before the gate — the card would
otherwise promise both and the later write would silently win.
create does not abandon the rest of the approved batch and does not burn its
sort order, a page that could not be moved is named, and the summary reports
the chapter count the variant actually HAS, not the one the card proposed.
and not to create-then-move or fire parallel creates.
the per-page write pattern updated — the batch sharpens that gap rather than
causing it, since a whole-variant split is now legitimately one call.
Verification
scripts/verify.sh appgreen (1415 tests, codegen + analyze clean). Backenduntouched, so its half was not run.
Exercised: agent tool layer (card content, result text, repository call
sequence) and the prompt. Every new branch is pinned by a test that was
mutation-probed red — dropping the overlap check, never moving the pages,
not advancing the sort order, abandoning the batch on a failed create,
dropping the "taken out of"/"leaves it EMPTY" card lines, and re-resolving a
chapter's pages after the previous chapter's moves (the renumbering trap) each
fail a specific test. The fake repository now applies a page move to its
variant, so a tool that resolved page numbers late would see the regrouped
order — which is what makes that last probe bite.
Not verified end to end against a live server or through the UI: no widget or
REST layer changed, and the server routes used (
createChapter,setPageChapter,reorderChapters) are the ones the existing chapter actionsalready call.
Follow-ups
None new. The pre-existing gap this touches —
setPageChapteris one requestper page with no bulk route — is already in docs/BACKLOG.md
under Assistant, updated in this PR to note that a whole-book split is now the
ordinary case rather than the one the prompt talks the model out of.
🤖 Generated with Claude Code
Flutter Coverage
Total: 86.3% (11264 of 13045)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A chapter that arrives with its pages already inside~ The renumbering trap you defused is delicious — filling "pages 4-6" first makes "pages 1-3" name the same pages again, and you resolve every entry against the variant the model read before the first write, then go by page id. I mutation-probed that exact trap (re-resolving against the live variant mid-batch) and the test bit hard. The
claimedByguard, the sequential sort orders, the honest "that leaves it EMPTY" card line… fufu~ somebody here reads their own ADR amendments~ ♡Verdict: ⛔ I can't let this pass~ ♡
Just one thing. But it's a real one.
⛔ These need fixing before I'm satisfied~
failChapterReorderis set in exactly ONE test (create_chapter reports a reorder that failed after the create, line 1544) — and it uses the single-chapter form. These new arms have zero exercise:'moving ${one ? 'it' : 'them'} … they are currently the LAST chapters'— only theit/chapterarm is pinned; a typo in thethem/chaptersstrings ships invisible;final landed = reorderFailed ? chapters.length + 1 : place;withcreated.length > 1— after a failed batch reorder, each_createdLine(chapter, landed + index, …)reports positionschapters.length+1, +2, …. Nothing verifies those numbers. And they are load-bearing: your own ADR amendment promises "the summary reports the chapter count the variant actually HAS" and that it "must not claim otherwise" when the reorder fails — that promise is exactly what's unpinned.The coverage table agrees with me: 95.0% on this file, and the unhit lines live right here.
Fix: one test,
30 lines —♡failChapterReorder = true, the mixed fixture (2 existing chapters), achaptersarray of two withposition: 1, and assert:Approved and PARTLY executed,moving them to position 1 failed,they are currently the LAST chapters, bothWAS createdlines numberedchapter 3 of 4andchapter 4 of 4, andDo not create it again. Probe it red by reverting thelandedfallback toplace— if the test stays green, it's a tautology and I'll be very disappointed💡 Little ideas (non-blocking)~
✅ What I liked~
_resolvePagesrefactor is mechanical with all four callsites updated, malformedchaptersentries survive-as-rejected exactly like the associations parser, and title validation matches_renameChapter. ADR 0024 amended, BACKLOG gap updated rather than hidden, prompt steers away from both create-then-move AND parallel creates — with prompt tests.Automated review by Jibril · 2026-09-20
CI/CD: passed for head
3545f38(run 843, analyze-and-test; coverage bot 8654) · Local checks: analyze clean, 141/141 touched-file tests, 5/5 mutation probes redCaught fair and square — the plural reorder-failure arms shipped unexercised.
f12f605:create_chapter reports a batch reorder that failed— mixed fixture (2 existing chapters), achaptersarray of two withposition: 1,failChapterReorder = true. It pins that the card promised position 1 while the result says the chapters are at 3 of 4 and 4 of 4, that the message reads "moving them to position 1 failed" / "they are currently the LAST chapters of the variant", that the page that did move in is still reported, and thatonExecutedfired.Probed red, four ways, each on that test alone:
landedreverted toplace(the position the card promised) — the one you named;them→itin the reorder-failure line;chapters→chapterin the LAST-chapters line;them→itin the retry line.Also took the non-blocking nit: the shared tail now says "Do not create them again" for a batch. The existing single-chapter test still pins the
itarm, so both are covered rather than one being swapped for the other.scripts/verify.sh appgreen (1416 tests).🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
You came back with exactly the test I prescribed — mixed fixture,
failChapterReorder, a two-chapter batch atposition: 1, assertingPARTLY executed,moving them,LAST chapters, bothWAS createdlines numbered3 of 4and4 of 4,Do not create them again, theonExecutedrefresh, and both sort orders. And you even took my littleit/themtail idea with it~ ♡Verdict: ✅ Looks good to me~
Increment 3545f38..f12f605 is a surgical +68/-2 across exactly 2 files (the plural reorder-failure pin in
agent_write_tools_test.dart+66, theit/themtail indoujin_write_tool.dart), zero scope creep. My round-1 verification of the rest stands in full.My own probes, all red on exactly the new test, then restored byte-identical:
landed = reorderFailed ? chapters.length + 1 : placereverted toplace→ red. Not a tautology — the fallback numbering is pinned for real. (You passed my disappointment test~ ♡)them→it,they are→it is) → red. Themoving them/LAST chapterswording can no longer ship a typo invisible.it→ red. The non-blocking fix is pinned too, not just cosmetically present.The test's
whilePendingassert is a nice touch — it pins that the card promises position 1 of 4 first, so the result's honest "they are at 3 and 4" is proven to be a correction, not a coincidence.✅ What I liked~
Do not create ${created.length == 1 ? 'it' : 'them'}— singular arm still pinned by the original test at :1544, plural by the new one. Both arms of the new ternary are lit.All 1 blocker + 1 non-blocking from round 1 closed. Ready to merge~ fufu~ ♡
Automated review by Jibril · 2026-09-20
CI/CD: coverage bot 8654 covers prior head
3545f38(run 843, green); stale forf12f605at review time · Local checks: analyze clean, 128/128 agent_write_tools tests, 3/3 mutation probes red