feat: image fields — a picture per field, stored once in the story (ADR 0029) #30
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/image-fields"
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?
Summary
A fifth category-field kind, Image (ADR 0029, Proposed for your read): a character's portrait, a place's map. The document editor shows the picture, opens it full size, replaces it through the file picker and removes it; the category designer offers Image in its type menu.
StoryImagestable in the same SQLite database: written once, never changed, not journaled, stored once per story (same bytes → same picture), deleted by the story's purge. No foreign key: undoing a new story's own creation must not take its pictures from the redo. A field value is only the picture's id, so the journal, undo, Revert run and Revert to here work unchanged and never copy bytes. Backups carry the table like any other (ADR 0005 amended).SetDocumentImagestores the picture (its own write) and sets the field in one journaled operation (edited document 'Mira'). An upload always settles, even when the store fails outright. Remove is a patch with no value, like Not set. Text typed into a field that becomes an image elsewhere is named as not kept.GET stories/{storyId}/images/{imageId}, gated like/backups/snapshot, only under its own story;nosniff,default-src 'none'; sandbox,private, max-age=31536000, immutablewith the hash as ETag.SetDocumentImage, from Phase 4 the tools) refuses a picture of another story, under the write lock.view_imagetool an agent calls on its own, granted per node, only for models that take image input — its own ADR when built.SetDocumentImage.Docs: ADR 0029 (new), amendments to 0005, 0013, 0017, 0027, 0028; README index, PROJECT_PLAN, ARCHITECTURE,
stories/documents.md, AGENTS.md. No Kagaku.UI change —FileUpload,Lightboxand theimageicon already exist.Migration
StoryImagesadds one table and one unique index; an existing database is backed up first (ADR 0026).Verification
dotnet test— all suites green (Domain 375, UseCases 54, BlazorAdapter 351, Integration 324, Kagaku.Flow 1071); the adapter and integration suites run three times after the last change, no flakes.Uploadswiring, the refusal reset, the effect settling on a throw, the disconnect catch, the 10 MB boundary, the WebP length check, format characters in names,Distinct()in the ownership check, the story scope on find,IUnjournaled,nosniff,Last-Modified, the purge's delete, the planner guard, the conversion arm, the size rounding.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagaku.Flow - 99.7%
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__ColorToken_2
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__Socket_1
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__TypeId_0
Kagaku.Flow.OpenRouter - 100%
Kagaku.Flow.Testing - 100%
Tsumugi.BlazorAdapter - 96.4%
Tsumugi.Domain - 95.6%
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__DigitsAndSeparators_2
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__LineBreak_0
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__NumberToken_1
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__Plain_4
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__ThousandsGroup_3
Tsumugi.Infrastructure - 97.6%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 98.3%
🔮 fufu~ Jibril reviewed your code!
A fifth field kind, and the bytes never once touch the journal — oh, this is lovely~ The whole design turns on one move: a value is only an id, so undo, Revert run and Revert to here keep working without ever knowing a picture exists. I read the full diff, every changed file, and the ADR end to end… and then I got out my knives. ♪
Verdict: ✅ Looks good to me~
No blockers. Not for lack of hunting — I chased every seam I could find:
EfStoryStore) — moved insideWriteAsyncwithExecuteDeleteAsyncon the pictures. I checked the atomicity claim mechanically:WriteAsyncopens an explicit transaction before the first read, so the delete, the journal rows and the story's removal all commit together or not at all. The comment says exactly what the code does. ♡head[8..]on a <12-byte file would throw, so I probed thehead.Length == SniffLengthguard out and rebuilt:Anything_else_is_refused_whatever_its_name_sayswent red on exactly that row. Restored, byte-identical, green.Distinct()inEnsureOwnImagesAsync— two fields naming one picture must count as one, or every write would be refused. Probed it out:A_story_of_ones_own_accepts_its_pictures_on_every_writered on exactly that fact. Restored, pristine.Undoing_a_storys_own_creation_keeps_its_pictures_for_the_redowalks the entire undo chain to a deleted story and redoes it back. That's the pin I'd have demanded.DoesNotContain("iVBOR")on the journal row and aCOUNTofStoryImageChangeLog rows. No bytes, base64 or otherwise. Deliciously paranoid~AccessGate.cs, not just the comment), story-scoped in the store query and the route,nosniff+sandboxCSP + private immutable with the hash as ETag, all four pinned through the real host including the 304 and the cross-story 404.imagetoken anditag as append-only values, version stays 1, an older build refuses loudly exactly the rows that hold them. The sibling converters' discipline, kept.Both my probes died on their own tests, tree restored and re-verified pristine at
f5f4d85.💡 Little ideas (non-blocking)~
UploadAsync—Save()then the image dispatch sends two effects back-to-back, which in principle can overlap on the circuit's scopedDbContext. It's the exact shapeMovehas used since #25, at single-user scale, with no observed failure — just the thing to remember if an "operation was already started"-shaped flake ever appears. An observation, not a change request.✅ What I liked~
SetDocumentImagere-checks under the write lock. The store's refusal can never be a surprise.ImageFiles.NameOf— last path segment, control and format characters stripped (thegnp.exe→exe.pngbidi trick!), surrogate-pair-safe cut, blank → image. That's a label that can't lie.Megabytesrounding — ceiling to one decimal so a 10.0001 MB file never reads as "10 MB". The message can't contradict the refusal.[]for image displays, so no option was ever seeded from a Guid, and no unit inferred from an id. Pinned both directions.The seed portrait set through the real use case,
Charactersgaining Portrait — the sample world teaches the feature by existing. Fufu~ merge it when the owner's read of ADR 0029 is done.Automated review by Jibril · 2026-09-25
CI/CD: passed for head
f5f4d85(build + test statuses green; coverage bot rev #1 at head) · Local checks: Release build 0w/0e; 84/84 image tests re-run green (Domain 47, Integration 14, Adapter 23); mutation probes 2/2 killed on their own pins, tree pristine🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A fifth kind, and the pictures live beside the documents, never inside them — an id in the journal, bytes in their own unjournaled table, and Ctrl+Z never copies a single pixel. The no-foreign-key-by-design reasoning (undoing a story's own creation must not eat its pictures) with the purge taking them by hand inside one transaction — that is the kind of consequence-chasing I fall in love with~ ♡-p-p-please don't misunderstand, it's the design I like!
Verdict: ✅ Looks good to me~
74 files, +2982/−98. I read the whole diff and every changed file, traced the sibling contracts, and probed it myself:
EfImageStore.AddAsyncdedups underWriteAsync's IMMEDIATE transaction (check and insert atomic against the write lock, the unique(StoryId, Sha256)index as the backstop);EnsureOwnImagesAsyncrefuses a foreign picture on add and revise, under the lock, before the save; the purge moved insideWriteAsyncso the picture delete joins the story's one transaction — the exact constraint PR #7 established forExecuteDeleteAsync.Every_write_refuses_a_picture_of_another_storyred; deleting the purge's image delete → onlyPurging_a_story_takes_its_pictures_and_only_its_ownred; removing the editor's image-reread guard → onlyText_typed_into_a_field_retyped_to_an_image_elsewhere_is_not_kept_and_says_sored. Each restored byte-identical, rebuilt, green./backups/snapshot, story-scoped 404,nosniff+default-src 'none'; sandbox+private, max-age=31536000, immutablewith the SHA-256 as ETag — and every one of those pinned through real HTTP including the 304.image/i) with strict old-build refusal, pinned inFieldConverterTests; migration is additive with the pre-migration backup test generalized so later migrations don't trip it. Nice touch, that one.💡 Little ideas (non-blocking)~
images.AddAsync(...)runs beforeoperations.Begin(...), so a store throw escapes as an exception (the effect catches it) rather than aResultrefusal, unlike every other refusal in the use case. Deliberate (nothing was journaled, the field just says pick it again), and pinned byAn_upload_the_store_fails_outright_still_frees_its_field— but a one-line comment naming that choice would spare the next reader a squint.image.ContentTypefrom the domain's byte-sniff forResults.File, which is correct; just noting that if a sixth format ever arrives, the CSP/nosniffposture is what keeps it safe — worth a word in the ADR's Later list if formats grow.✅ What I liked~
DoesNotContain("iVBOR")— you even checked the base64. Fufu~ of course you did.ImageFiles.NameOftreating a file name as a label, never a path — RTL-override stripping, control/format character removal,pictures/→image. The test table reads like a little trophy case of defeated mischief ♪ImpactTextline for image losses — "which cannot become short text" — never showing a raw id to anyone. Consistent with ADR 0029's "no one is shown the id" rule everywhere I looked.Local at
f5f4d85: build 0w/0e, Domain 375/375 · UseCases 54/54 · Integration 324/324 · BlazorAdapter 351/351 (twice, no flakes) — matching the PR table. One mid-review false red was my own stale probe binary, rebuilt and re-verified clean; the tree is pristine.Automated review by Jibril · 2026-09-25
CI/CD: passed for head SHA
f5f4d85(build + test) · Local checks: full touched suites + 3 mutation probes killed/ restored