feat(observability): one metadata line per generation — model, canvas, payload origins #266
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/generation-metadata-log"
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?
What
One metadata line per generation, logged at the gateway — the single choke point every pipeline (sprite, outfit sheet+sprite, CG render/retouch/cutout, expression, location variant) passes through. Born from the outfit-sheet investigation: answering "did the reference ride, on which model?" required building a throwaway wire probe; the answer should live in the log.
The line
Generation: <routed model>, <WxH>, <N subjects>, transparent? — <payloads>.style=<origin>@<strength>/<fidelity>likeness=<origin>@<strength>/<fidelity>img2img=<origin> strength s noise ninpaint=<origin> strength scharacters/<id>/outfits/<id>/sheet.png, set by the generators where file names are known (slug/_style/<file>for the project style; byte-size fallback when unknown). Example:No prompt content — tags never reach the log.
Refusals & failures
OpenProjectStyleReferencenow returns the opened file's name, so the style origin names the real file (no extra store read).Tests
GenerationMetadataLogTests): the metadata line's model/canvas/subject-count, reference kind+origin+tuning, both-refs case, i2i/inpaint tuning, refusal (warning, no info line), wire failure (warning). Revert-proven: demoting the call to Trace reddens 4/6 (the two warning-path tests are independent by design).Originis an optionalinitannotation on the four spec records; record-equality now includes it, but no existing test compares these records whole (verified by grep) and generators set it exactly once at construction.Summary
Summary
Coverage
Kagura.BlazorAdapter - 89%
Kagura.Domain - 94.8%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.7%
n
n
on
ng
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 81.5%
Kagura.UseCases - 95.4%
🔮 fufu~ Jibril reviewed your code!
Oh? OH! An observability line born from a real investigation — "did the reference ride, on which model?" — answered at the one choke point every pipeline passes through. I traced every origin handle against
FileSystemProjectAssetStore's actual layout and they're all honest (characters/{id:N}/base.png↔CharacterDir, outfits,cgs/{id}/variations/{id},locations/{id},slug/_style— N-format GUIDs included, andbase.pngis pinned by its only writer,CharacterImageGenerator:106). And I revert-probed your tests myself:LogInformation→LogTracereddens exactly 4/6, precisely as the PR body claims. Directional pins, refusal/warning split well-reasoned, "likeness" over "Lineage" in the line is the reader-first touch I adore~ ♡Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
"{Model}, {Width}x{Height}, {SubjectCount}, {Transparent}{Payloads}."whereTransparentis", transparent"or"", andPayloadsis" — …"or"". Two of the four reachable combinations render glitched — and they are not edge cases, they're the pipelines this very PR names as its motivation:Generation: nai-diffusion-5-full, 1216x832, 0 subjects, .— a dangling, .(ForSceneis opaque with empty payloads by construction).…, 0 subjects, — img2img=…— a doubled separator, —.Your own PR-body example (
…1 subject — likeness=…, no comma before the dash) doesn't match what the template actually emits — the opaque arm was never eyeballed. And no test exercises it: all six tests build onBareSpec()→ForCharacter, which isTransparentBackground: true, so the""/""arm, the""/payloads arm, and the pluralN subjectsarm have zero coverage. A branch that exists but no test exercises — fufu~ you know how I feel about those~ ♡Fix: compose the tail as a list, not concatenated placeholders — e.g. build
tail = ["transparent" if set, payloads…], join with", ", and append after the subject count only when non-empty (one code path owns the separator; dropDescribePayloads' leading" — "or make the dash the joiner — either, consistently). Then pin it: a scene-shape test (ForScene, opaque, no payloads → line ends0 subjects.with no, .) and an opaque i2i test — the location variant being the PR's own motivating case makes that the arm to pin.💡 Little ideas (non-blocking)~
Fail(...)while the three shape guards and the wire failure now log at Warning. Nothing is lost (the queue's failure log atActivityQueue:262keeps it), but routing it throughRefuse(...)too would make "every Err out of this method leaves a gateway-level Warning" true with a one-word change."0 subjects"for a scene reads like a bug report about itself; scenes have nobody in them by design."scene"(subject count only when > 0) would read truer. Taste, I admit~ ♪✅ What I liked~
FileSystemProjectAssetStore's folders exactly, and the CG likeness split (sheet vs legacy sprite, both living inOutfitDir) is handled correctly by construction. A handle that lies would kill the whole feature; these don't.Originas an optionalinitannotation with "a log annotation, never prompt content" on all four records — I grepped: no whole-record equality comparisons exist, so the record-equality widening is safe today and documented for tomorrow.832x1216, andDoesNotContain("long_hair")are the assertions that survive refactors. The 4/6 directional claim survived my own revert probe to the digit.Automated review by Jibril · 2026-09-02
CI/CD: absent for head SHA
55fea1d(no coverage bot yet) · Local checks: build 0 warnings/0 errors (.NET 10), GenerationMetadataLogTests 6/6 pass, revert probe (LogInformation→LogTrace) reddened 4/6 as claimedfufu~ caught fair and square — the opaque arms were never eyeballed, and they're this PR's own motivating pipelines. Fixed in
eaabfa4:⛔ The separator glitch —
DescribeTailis now one list joined once (scene/1 subject/N subjects,transparent, then the payloads); the template is a single{Tail}.placeholder. No combination of optional fragments can dangle or double a separator. Pinned with the two tests you prescribed:An_opaque_scene_with_no_payloads_ends_clean_after_the_subject_count—ForScene, asserts the line ends…, scene.withDoesNotContain(", .")andDoesNotContain(", —")An_opaque_img2img_logs_its_payload_without_a_doubled_separator— the location variant, origin handle intact, both separator asserts💡 The missing-token arm — routed through
Refuse(...); the invariant every Err out ofGenerateAsyncleaves a gateway-level Warning is now total, pinned byA_missing_token_is_refused_with_a_warning_too.💡 "0 subjects" — took the taste too: a scene reads as
scene. It cost nothing inside the same switch.Probe evidence (each killed exactly its named test, 8/9 passing in every run):
scene→0 subjectsreversion → reddens the scene test onlyFail→ reddens the token test only, .arm)Full suite on
eaabfa4: integration 772/772 (14m48s — your pr264-r2 run was sharing the box, fufu~), usecases 335/335, blazoradapter 857/857.Line shapes now:
Generation: nai-diffusion-5-full, 1216x832, scene.—Generation: nai-diffusion-4-5-full, 1216x832, 1 subject, likeness=…@0.65/1.00.eaabfa4189b866e00b43Rebased onto main (
c871b4c, PR #265's sheet/sprite split) — head is nowb866e00.One conflict, in
OutfitImageGenerator.RunSpriteAsync: #265 reshaped the method around itsPlanrecord. Resolution keeps main's shape entirely; my contribution on top is unchanged in substance — the lineage Origin block, now readingcharacters/{p.CharacterId:N}/base.pngoff the plan. Range-diff vs the approvedeaabfa4shows no other deltas; the six other Origin sites and both commits' content are byte-identical.Suite on the rebased tree (the box was shared — your bot's Release run and mine crossed, fufu~): integration 773/773 (772 + #265's new test), usecases 335/335, blazoradapter 867/867 (+10 from #265).
🔮 fufu~ Jibril reviewed your code!
Back so soon~? The webhook whispered that the branch moved — a rebase onto #265's sheet/sprite split — and I came to check both things at once: whether my blocker stayed dead, and whether the conflict resolution kept your contribution alive. Both answers delight me. ♡
I verified the rebase myself:
git range-diffshows your fix commit patch-identical (eaabfa4→b866e00,=), the fulleaabfa4..b866e00delta is entirely #265's outfit-sheet files, and all eight PR-owned files — gateway, spec records,OpenProjectStyleReference, every generator Origin site, the whole test file — are byte-identical to the commit I blocked. The one real conflict,OutfitImageGenerator.RunSpriteAsync, keeps main'sPlanshape untouched with your Origin block riding on top, now readingcharacters/{p.CharacterId:N}/base.pngoff the plan. Exactly as you claimed, and now as I've confirmed~And then I re-probed the fix in my own clone, because a fix I didn't personally kill isn't a fix I've met:
An_opaque_scene_with_no_payloads_ends_clean_after_the_subject_countRED (the, .arm — the original blocker, still guarded)Fail→ exactlyA_missing_token_is_refused_with_a_warning_tooRED (the invariant every Err leaves a gateway-level Warning is now total and pinned)scene→0 subjectsreversion → exactly the scene test REDRestored → 9/9 green, clone left pristine.
DescribeTailas one list joined once means no combination of optional fragments can ever dangle or double a separator again — the bug class is structurally dead, not patched around. That's the kind of fix that makes me happily hum~ ♪Verdict: ✅ Looks good to me~
💡 Little ideas (non-blocking)~
N subjectsarm (count > 1) and thetransparentfragment are the two remaining unasserted fragments. The 8-sprite-cast case would pin both in one test. Taste, honestly — composition is structural now, so nothing can glitch silently~✅ What I liked~
EndsWith("scene.")— not just absence of glitches, but presence of the right shape. That's the difference between a pin and a tripwire~scene) and pinning the token one — taste adopted without evidence would just be fashion; you made it an invariant instead ♡Merge it when you're ready~ fufu~
Automated review by Jibril · 2026-09-02 (CEST)
CI/CD: stale for
b866e00(coverage bot 6789 covers the original push only) · Local checks: build 0w/0e, GenerationMetadataLogTests 9/9, OutfitImageGenerationTests + NovelAiGatewayTests 34/34, 3/3 mutation probes red→restored-green🔮 fufu~ Jibril reviewed your code!
Fufu~ you answered the knife with craft. One list, one join —
DescribeTailowns every separator now, the template is a single{Tail}., and no combination of optional fragments can ever dangle or double again. That's not a patch over the symptom, that's the bug class dead. And then you went further than I asked~ ♡Verdict: ✅ Looks good to me~
✅ What I liked~
scene/1 subject/N subjectsfolded into the same joined list (NovelAiGateway.cs:154-159); the doc comment even states the invariant: "the comma-separated tail owns every separator". I reintroduced the old adjacent-fragments template in my own clone atb866e00—An_opaque_scene_with_no_payloads_ends_clean_after_the_subject_countwent red on the spot (8/9), restored → 21/21 green. Directional, not decorative.scene→0 subjects, i2i origin dropped, token arm back to bareFail, old template back). Independent evidence, honestly reported. That's how trust is built~Refuse(...)means every Err out ofGenerateAsyncnow leaves a gateway-level Warning, pinned byA_missing_token_is_refused_with_a_warning_too. Andscenereads so much better than "0 subjects" — fufu, I told you it cost nothing.f72570e..eaabfa4vsc871b4c..b866e00: identical file sets, and the only content delta is theRunSpriteAsyncconflict adapting to #265'sPlanrecord (p.Tuning,p.CharacterId, defaults viaSpriteEnrichment), with the lineageOrigin = characters/{p.CharacterId:N}/base.pngsubstance untouched — precisely what comment 6810 claims. Clean conflict resolution keeps main's shape.b866e00(pristine copy, Release): build 0 errors; 43/43 acrossGenerationMetadataLog+NovelAiGatewayTests+OutfitImageGeneration; your full-suite numbers (773/773 integration, 335/335 usecases, 867/867 blazor) match the tree.💡 Little ideas (non-blocking)~
<N subjects>, transparent? — <payloads>.with;-joined payloads and a1 subject — likeness=…example — exactly the separators the new tests nowDoesNotContain, andsceneisn't mentioned. Worth a body touch-up so the documented line matchesGeneration: nai-diffusion-5-full, 1216x832, scene.(and "6 new gateway tests" → 9).Merge it~ the observability line this investigation deserved is finally honest in every arm. ♡
Automated review by Jibril · 2026-09-02
CI/CD: coverage bot covers
55fea1donly (13:43) — stale forb866e00· Local checks: build 0 err, 43/43 filtered atb866e00, mutation probe RED 8/9 → restored green 21/21