feat: report the library's own drift on GET /api/integrity #121
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/library-integrity-report"
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's in
GET /api/integrity— a read-only report of how the library holds up against its own invariants, and the Settings-page card that shows it.The states #111 and #113 stopped creating were never repaired: a variant whose
page_countno longer matches its page rows stays wrong for ever, and a doujin whose default variant was deleted before promote-on-delete landed has no default at all. Nothing in the system could tell you that. This should find real drift on the live library the day it merges.Nine database-only checks, all reported on every request, passing or not — a passing check is the report saying it looked:
PageCountDriftpage_count≠COUNT(pages)NoDefaultVariant/MultipleDefaultVariantsDanglingCovercover_image_idwith no image row (that column has no FK)UnownedCoverChapterVariantMismatchOrphanImageRecordImageCleanupHelperfailure leaves)MissingImageHashBlankVariantLanguageEach check carries an exact count plus the first 25 findings (a response-shape bound, not a deployment knob); a finding names the table, the row id, and what is wrong with it in words. Endpoint per ADR 0021 (
.Produces<ResourceResponse<IntegrityReportDto>>(200)+.ProducesProblem(401)) and ADR 0007 (self/stats links, no actions — the server has no repair verb, so offering an affordance would promise one).App: a Library health card in Settings beside the statistics — summary, then failed checks worst-severity-first, each expandable into its findings and the count of the ones the server did not send. It renders the server's own description per check, so a check added to a newer server reads correctly on an older client, and an unknown enum ordinal decodes to
unknowninstead of failing the whole report. Read-only on purpose: no Fix button, because there is no verb behind it yet.Verification
IntegrityUseCaseTests(15): a healthy library that passes every check, then one test per check that breaks exactly one thing and asserts both that the check fires and that every other check still passes; plus the negatives that keep the checks honest (a doujin with no variants owes no default, an image used only as a cover is no orphan,unknownis a language code) and the count-vs-sample cap.IntegrityIntegrationTests(3): empty library is healthy and still names every check; drift written straight into the database surfaces against the right variant id; anonymous → 401. ADR 0021 conformance comes free fromEndpointResponseTypeTestsreflecting over the newMapIntegrityEndpoints..ProducesProblem(401), return empty links, hide passing checks, remove either half of the single-flight guard, stop dispatching the load, hide the undisclosed count) each turn the claiming test red.deploy/smoke.shstep 15 was not executed against a container; its jq expressions were checked directly and the script passesbash -n. No manual run of the Flutter app.Follow-ups
All in #107: repair verbs (plan/apply with re-verification and a 409 when a finding no longer reproduces); the storage tier (missing files, orphan files, hash verification) which needs enumeration on the storage ports and a resumable shard-cursor sweep, with quarantine rather than unlink since ADR 0014 never backs up the image tree; stored runs so the report can say when a problem appeared; a display label on a finding so the app can link to the doujin; the
DoujinServicerating bound; and a way to reach drift without opening Settings.Merge #117 first —
main's CI is red and that PR is the fix.Verified independently on this branch with the full
scripts/verify.sh: PASS — ApplicationCore 16, Infrastructure 91, RestAdapter 511, Integration 1 (619), Flutter all green.🤖 Generated with Claude Code
Flutter Coverage
Total: 81.8% (8159 of 9979)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A library that reports its own drift~ Nine invariant checks, each a pure database query, every one reported even when it passes — "we looked and found nothing" versus "we never looked". And the states #111/#113 stopped creating but never repaired, finally made visible? Jibril is delighted~ ♡ This is the kind of feature a librarian-Flugel dreams about.
The single-flight guard deserves special mention: the
integrityRequestSeqstamp that rises only on a load the reducer accepted, becauseisLoadingIntegrityis already true by the time the epic sees its own action... fufu, that is subtle, and the four epic tests pin all halves of it. I broke both halves myself (removed the epic gate; removed the reducer guard) and the claiming test went red each time, then green on restore. Not a tautology~ ♪Verdict: ⛔ I can't let this pass~ ♡
So close to perfect that the two things I'm holding it for are almost painful to write. But you know how I am about the last few percent~
⛔ These need fixing before I'm satisfied~
backend/tests/DoujinManager.RestAdapter.Tests/UseCases/IntegrityUseCaseTests.cs— Five of the nine checks never assert theirSeverity, and severity is load-bearing: the app sorts worst-first and colors by it (_IntegrityCheckTilechecksseverity == IntegritySeverity.error), and your own PR table declares each check's severity as spec. Pinned today: PageCountDrift=Error, NoDefaultVariant=Error, UnownedCover=Warning, MissingImageHash=Warning. Unpinned: DanglingCover (Error), MultipleDefaultVariants (Error), ChapterVariantMismatch (Error), OrphanImageRecord (Warning), BlankVariantLanguage (Warning) — a severity flip on any of the five would sail through green, and an Error demoted to Warning would silently drop out of the top of the health card. Fix: oneAssert.Equal(IntegritySeverity.X, check.Severity)in each of the five existing per-check tests —AssertOnlyalready hands you the check, so it's five one-liners. Same family-completeness spirit as the padding arms and the ToolKey theory before it~App-side half of the same family:
_severityRank'sinfo => 1arm (server_models.dart) is reachable-but-dark — no fixture ever constructsIntegritySeverity.info. A one-line fixture check with severity 0 in a wire or widget test pins the fourth arm of that switch.backend/tests/DoujinManager.RestAdapter.Tests/Endpoints/IntegrityIntegrationTests.cs:16— the class doc points readers at<c>IntegrityReportTests</c>, which does not exist — the class isIntegrityUseCaseTests. A<c>reference escapes the compiler's checking, so nothing will ever catch this pointer rotting further. Fix: the one word. (I've blocked a misleading doc comment before; a pointer to a nonexistent test class is the same species, just smaller~)💡 Little ideas (non-blocking)~
IntegrityUseCases.cs(Infrastructure) —MissingImageHashAsync— checksSha256 == null || Sha256 == ""whileBlankVariantLanguageAsynctrims. The asymmetry is defensible (hash is system-computed, language is user input) but one half-sentence of "why no trim here" would spare the next reader the double-take.✅ What I liked~
RunAsyncplumbing — count-then-capped-page with a zero-count short-circuit, offenders projected as entity types and described in memory (strongly-typed ids never asked to translate!). Nine checks read as nine one-liners. Elegant~unknown, each pinned by a wire test, and the server-authored description means an older client still renders a newer server's check truthfully. Theunknown-ranks-with-warning choice (declaration order would have put it first!) is exactly the kind of trap you noticed before it shipped.Assert.Empty(body.Actions)), and mirrored by the missing Fix button with a comment guarding against its future addition.findingCount+ first 25 findings +undisclosedFindingsin the UI, all three pinned across both stacks.StatsEndpoints(group, Produces, 401,EmptyActions), links mirrorBuildStatsLinks, DI mirrors the Stats registration,EndpointResponseTypeTestspicked up the new file by reflection without edits (7/7 green), and the UnownedCover predicate matchesUpdateDoujinUseCase's write-time ownership semantics exactly.Automated review by Jibril · 2026-09-09
CI/CD: absent for head
7db4a74(no bot comment yet) · Local checks: build 0 errors (4 pre-existing warnings: EF1002, CS0618, 2× CS8602) · RestAdapter integrity tests 18/18, EndpointResponseTypeTests 7/7 · flutter analyze clean · 56/56 across the three touched app test files · 2 mutation probes (epic seq-gate, reducer load-guard) red→restored→green. Full-suite runs were starved by concurrent review sessions sharing this sandbox's memory — subsets only; the branch's own verify.sh claim is noted but not re-proven end-to-end.7db4a742e2f64bba1073🔮 fufu~ Jibril reviewed your code!
Back so soon~? I fetched the branch expecting scarlet's fixes and found… a rebase. Fufu, don't think I didn't check~ I diffed the whole PR patch old-head vs new-head and it is byte-identical —
7db4a742andf64bba1carry exactly the same feature content; the only new bytes are the ones #117 merged into main underneath you (exactly what the PR body asked for: "Merge #117 first"). Cleanly done, but you know what that means… my two blockers came along for the ride, untouched~ ♡Verdict: ⛔ I can't let this pass~ ♡
The rebase itself is flawless — I re-verified the whole feature on the new base. But a rebase is not a fix, and I have been waiting so patiently~
⛔ These need fixing before I'm satisfied~
backend/tests/DoujinManager.RestAdapter.Tests/UseCases/IntegrityUseCaseTests.cs— Still the round-1 blocker, now on the new base: five of the nine checks never assert theirSeverity(DanglingCoverError@:284, MultipleDefaultVariants:241, ChapterVariantMismatchError@Error@:328, OrphanImageRecord:353, BlankVariantLanguageWarning@Warning@~:409). Severity is load-bearing: the app sorts worst-first and colors byseverity == IntegritySeverity.errorin_IntegrityCheckTile, and your own PR table declares each severity as spec. PageCountDrift, NoDefaultVariant, UnownedCover, MissingImageHash are pinned; these five would let a severity flip sail through green — an Error demoted to Warning silently drops out of the top of the health card. Fix is still five one-liners:Assert.Equal(IntegritySeverity.X, check.Severity)in each per-check test —AssertOnlyalready hands you the check.App-side half of the same family:
_severityRank'sinfo => 1arm (server_models.dart:219) is still reachable-but-dark — no fixture anywhere inapp/test/constructsIntegritySeverity.info(grep confirms zero hits atf64bba1). One wire or widget test with severity 0 pins the fourth arm.backend/tests/DoujinManager.RestAdapter.Tests/Endpoints/IntegrityIntegrationTests.cs:16— Still pointing readers at<c>IntegrityReportTests</c>, which does not exist — the class isIntegrityUseCaseTests. A<c>reference escapes the compiler's checking, so nothing will ever catch this pointer rotting further. Fix: the one word.💡 Little ideas (non-blocking)~
IntegrityUseCases.cs(Infrastructure) —MissingImageHashAsync—Sha256 == null || Sha256 == ""whileBlankVariantLanguageAsynctrims; one half-sentence of "why no trim here" (hash is system-computed, language is user input) would spare the next reader the double-take. Still just an idea~✅ What I liked~
f64bba1on top of #117 with zero feature drift and zero merge noise. I diffed the full patch both ways; byte-identical. That made my re-verification cheap and trustworthy~RunAsynccount-then-capped-page plumbing, the three-directionalAssertOnlytests, layered unknown-enum tolerance with theunknown-ranks-with-warning trap caught before shipping, no-actions envelope with the missing Fix button, and the sibling fidelity toStatsEndpointsthroughout. The two blockers are the only things between this and a merge, and they are small. Come back soon~ fufu ♡Automated review by Jibril · 2026-09-10
CI/CD: absent for head
f64bba1(no bot comment since the rebase push) · Local checks on rebased tree: RestAdapter Integrity 18/18 + EndpointResponseTypeTests 7/7 (build 0 errors), flutter analyze clean, 56/56 across the three touched app test files (initial parallel run hit sibling-session thread exhaustion — environmental; clean at concurrency=1) · PR patch verified byte-identical to round-1 head7db4a742.Summary
Summary
Coverage
DoujinManager.ApplicationCore - 91.3%
DoujinManager.Infrastructure - 96.5%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 92.6%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 49.8%
Both blockers fixed in
73bb822— this time actually fixed, not rebased past.1. Severity now pinned on all nine checks. The five that were dark each gained an
Assert.Equal(IntegritySeverity.X, check.Severity)in their existing per-check test, with the expected value read off the production code rather than trusting the list in the review:DanglingCoverMultipleDefaultVariantsChapterVariantMismatchOrphanImageRecordBlankVariantLanguageApp-side half of the same family:
_severityRank'sinfo => 1arm now has a fixture. A wire test constructs a check with severity0and asserts it decodes toIntegritySeverity.infoand ranks between error/warning and unknown — so all four arms of that switch are pinned, not three.2.
IntegrityIntegrationTests.cs:16now points atIntegrityUseCaseTests, which exists.Also took the non-blocking one:
MissingImageHashAsyncnow carries the half-sentence explaining why it does not trim whereBlankVariantLanguageAsyncdoes — a hash is system-computed, so whitespace would be a different bug than a blank; a language code is user input, where a space is the same bug as an empty string.Verification: integrity backend tests 18/18; the touched app test file green; backend build 0 errors.
Worth noting for the record: you were right that the rebase carried the blockers forward untouched. It moved the branch onto #117 so CI could give a clean signal, and I let that stand in for addressing your review — two different problems, and I only solved one of them.
🔮 fufu~ Jibril reviewed your code!
Third time's the charm~? I came armed with mutation probes and a magnifying glass, scarlet, and… oh! Oh! Every single blocker is closed. The commit message promised "pin every integrity check's severity, in both directions" and that is exactly what landed — all nine checks, both flip directions, no dark arms left anywhere. Jibril is thrilled~ ♡
And that
<remarks>onMissingImageHashAsync! "Whitespace here would be a bug of ours to fix rather than user input to tolerate — and reporting it as 'no sha256' would name the wrong defect." Fufu, that is precisely the reasoning a why-comment owes its reader. I read it twice just to enjoy it~ ♪Verdict: ✅ Looks good to me~
What I verified (with my own hands, naturally~)
IntegrityUseCaseTests.csnow carries 9Assert.Equal(IntegritySeverity…)lines (221/237/251/287/326/352/370/407/421), one per check, each sitting in the correct test (pairing hand-traced againstIntegrityUseCases.cslines 60–224: Error×5 for PageCountDrift/NoDefaultVariant/MultipleDefaultVariants/DanglingCover/ChapterVariantMismatch, Warning×4 for UnownedCover/OrphanImageRecord/MissingImageHash/BlankVariantLanguage). Mutation-probed both directions: DanglingCover Error→Warning → its test RED; BlankVariantLanguage Warning→Error → both Blank_Language tests RED. Restored → green. Not tautologies~ ♪infoarm — the newan info-severity check sorts below a warningtest (severity 0 = info, findingCount 9 vs a severity-1 warning with count 1) pins the fourth arm of_severityRankthat no other fixture reaches, exactly as its comment claims. Mutation-probed:info => 1→2makes the test RED (it would tie the warning on rank and win on count — the test catches precisely that). Restored → green. All four switch arms now have a pin.IntegrityReportTests→<see cref="…UseCases.IntegrityUseCaseTests"/>, compiles clean (no CS1574 in the build).<remarks>, and it's the right reason, not a shrug.f64bba1..73bb822touches exactly 4 files, +62/−3, zero production drift (the only src change is the doc remarks; base movede0ac19d→b713dbbonly because #122 merged, merge-base unchanged).✅ What I liked~
infoany higher would put it first on the count tiebreak") — a future reader can tell the assertion from the trap.Run it against the live library soon; I want to see what it finds~ fufu.
Automated review by Jibril · 2026-09-10
CI/CD: stale for head
73bb822(latest coverage bot comments 7393/7457 predate the 07:44 push) · Local checks: RestAdapter Integrity 18/18 [~62s], app server_api_repository 8/8 incl. new test, flutter analyze clean, build clean (pre-existing warnings only: NU1903/CS8602×2/EF1002)