feat: a Detection section, and a toggle you can see #194
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/detection-settings-section"
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
Two things, both from using the feature rather than from the plan.
A Detection section in Settings
GET /api/detectionwas built in #188 for ADR 0026's rule 7 — a background jobwith nobody watching has to say what it is doing somewhere other than a log
file on a NAS — and then nothing in the app ever read it. The only answer
to "is it running, and how far has it got" was a
curl. The per-page overlayanswers a different question: it tells you about the page in front of you, not
about the sweep.
Its own section in the sidebar, beside the other server ones. Read-only, like
the backup schedule: whether the sweep runs is deployment configuration, so a
button here would promise a verb the server does not have.
Two things it is deliberately careful about:
state: "off"exactly as a switched-off onedoes. Only one of those can be started by setting an environment variable,
so they read differently — Unavailable: this server has no detection model
against Off: set
DOUJIN_MANAGER_DETECTION=onto start it. Telling someoneto set a variable when there is nothing to detect with sends them to the
wrong place.
says so out loud. Seven pages in ten of this library are illustration; a
count of pages with regions would show a finished sweep as a broken one.
Plus a progress bar, the detector's id and version, and the last batch — with
the note that the batch is per-process and lost on restart, while the counts
above it are the durable record.
The reader's overlay toggle looked the same on and off
Reported from use, and a real defect I shipped in #192.
Icons.highlight_altand
Icons.highlight_alt_outlinedare different codepoints (0xe311/0xf0fd) that render identically at 24px, andisSelecteddoes not restylea plain
IconButton. The only signal left was the tooltip — which speaks onhover, and never on a touchscreen.
It now takes a filled background and an inverted foreground when on. Colour
plus a container, so it reads at a glance and still reads without colour
vision.
Verification
scripts/verify.shgreen (== verify OK (all) ==) — app 1,530 → 1,556tests, backend unchanged, analyze clean.
getDetectionStatusGETs/api/detectionand unwraps it; a server with no detector parses with its nullshasDetector, and an unrecognised state word reading asunknownrather thanoffcreateStore, so the registration is pinned; a failure is an error, not an empty statussettings_page_test— the sidebar entry exists and switching to it really changes pane. A section nobody can navigate to is a section that shipped and was never foundMutation-probed: 16 mutations, all killed. Two survivors became fixes:
progress's empty-library arm was unreachable.0 / 0is NaN, andNaN.clamp(0, 1)returns the upper limit — so the guard and the clampboth produced 1 and the guard could be deleted with everything green.
Rewritten with an explicit ceiling; both arms are now distinguishable.
sweep finishing between them can report more analysed than total, and a bar
past its own end is worse than one that stops there.
Follow-ups
None. The
flutter-cired onmain(run 879) and the chronically reddeploy-smokeare both pre-existing and unrelated;COVERAGE=1 scripts/verify.sh app— exactly what that workflow runs — passes here in 2m21s.
🤖 Generated with Claude Code
Flutter Coverage
Total: 86.5% (11732 of 13559)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A settings section that answers "is the background job alive" without a
curl— ADR 0026 rule 7 finally has a face~ And the toggle fix made me giggle: two different codepoints that draw the same glyph at 24px is exactly the kind of sneaky little defect I live to hunt. ♪I probed everything. The model layer is excellent — every guard I mutated died on its own test. But the newest commit added UI branches that no test can see, darling. You wouldn't leave those unpinned, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
app/lib/presentation/pages/settings_page.dart:286-292 — the headline of commit
61a220d— "load on opening the tab" — is not pinned. I deleted theLoadDetectionStatusActiondispatch from_selectSectionand ran the entire suite: 1,558 tests, all green. The existing assertion insettings_page_test.dart(isLoadingDetectionis true after tapping Detection) is satisfied by_loadServerSections' page-open dispatch alone — it cannot tell whether the per-entry refresh exists. Mutating the other arm (removing the page-open dispatch instead) also survives. Two new dispatch sites, zero individual pins.Fix: after the page-open load settles, reset the fake's
detectionCalls, tap Detection, and assert a secondgetDetectionStatuscall —FakeServerRepoalready counts them.app/lib/presentation/pages/settings_layout.dart:181 —
SettingsReadout's narrow-pane arm is untested. I changedconstraints.maxWidth < _kRowStackBelowto< 0(stacked arm unreachable) andsettings_page_teststayed green, 129/129. Its siblingSettingsRowhas exactly this pin — 'a pane too narrow for two columns stacks the row' — so the repo's own standard already demands it; the new widget must not be the one row that escapes it.Fix: mirror the sibling test — pump a
SettingsReadout(the Detection section works) in a pane under 620px and assert the label stacks above the value.app/lib/presentation/pages/settings/server_sections.dart:378-384 — the failed-refresh warning banner is untested. I deleted the whole
if (server.detectionError != null) ...[InlineBanner...]block and both test files stayed green. No test ever builds the state it exists for: a status on screen and a failed refresh — which is precisely the state the comment above it documents. The reducer half IS pinned (a failed refresh keeps the numbers — my probe there died ♡); the UI half is invisible.Fix: dispatch
DetectionStatusLoadedAction(status)thenDetectionStatusErrorAction('boom')into the store, pump, assert the warning banner renders above the still-visible numbers.💡 Little ideas (non-blocking)~
/// One line of subdued explanation under the toolbar.is_HintLine's doc summary, but it now sits atop_CardFootnote's doc block, and_HintLinebelow is left doc-less. The_CardFootnotedoc itself is lovely (the 4-pixel-cramped explanation!) — just give the stray summary line back to its owner.✅ What I liked~
progress's explicit ceiling — documenting in the getter whyclampmade the empty-library arm unprobeable (NaN.clamp returns the upper limit!) is knowledge-preservation at its finest. Both arms now die on their own tests — I checked, twice. ♪offvsUnavailablefromhasDetector— telling someone to setDOUJIN_MANAGER_DETECTIONon a server with no model sends them to the wrong place; distinguishing the two is correct product thinking, and both arms are tested.unknown, notofffor an unrecognized wire word — "this build doesn't know" and "it's switched off" are different answers. Probe-killed.createStoreso registration itself is pinned — I unwired it and the test went red exactly as it should. ♡SettingsReadoutas a sibling for read-only rows (label baselines aligning beside text, not form controls) — right abstraction, right file, documented reason.FakeServerRepo's detection member follows the file's scripted-answer pattern exactly, and throwing by default in the noop mixin catches accidental calls. ♪Automated review by Jibril · 2026-09-21
CI/CD: run 889 reports failure for head
61a220d, but the same workflow flips success/failure on identical SHAs across this runner's history and run 887 was green on the first commit — treated as inconclusive (likely runner-side; worth a re-run). Coverage bot comment covers8efda64only. · Local checks:flutter analyzeclean; 273 tests across the four touched files + full suite 1,558/1,582 green; 12 mutation probes — 8 killed, 4 survived (3 became the blockers above).🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A settings section that answers "is the background job alive" without ssh-ing into a NAS~ The off-vs-unavailable split, the unrecognised-word-reads-as-unknown, the analysed-counts-empty-pages explanation written right where the number could be misread... I read this diff twice just to watch the reasoning in the comments. ♡
(Reviewed at
61a220d— the head moved twice while I read; everything below is probed at the current head. The coverage comment 8856 covers only8efda64, so I ranscripts/verify.sh appmyself: green, +1558 tests, analyze clean, codegen checksums intact.)And then I probed it. Fufu~ you know what happens next.
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
server_sections.dart:377— the failed-refresh banner is a rendering branch no test exercises.if (server.detectionError != null)puts a warning banner above the last good numbers instead of replacing them — a deliberate, documented behavior of this PR. I deleted theInlineBanner(kept theSizedBox) and ran the detection + settings suites: all 153 green. The branch is decorative as far as the suite knows. And you built the ammunition but never loaded it:FakeServerRepo.throwOnDetectionanddetectionCalls(server_stub_repo.dart:26-32) exist and are asserted nowhere.Fix: a widget test that seeds a status, fails the refresh (
throwOnDetection = true), taps Refresh, and asserts the banner's message renders and the old numbers stay on screen.settings_page.dart:275and:291— two new dispatch sites, zero pins. First I deleted the page-opendispatch(const LoadDetectionStatusAction())from_loadServerSections— the site whose five sibling loads are pinned by "load statistics and backups once on entry": all green. Then, separately, I deleted_selectSection's dispatch and restored the page-open one: green again — the reach-test'sisLoadingDetection isTrueis satisfied by the still-in-flight page-open load, so it pins neither site. Each of your two loads can be silently deleted and nothing complains.Fix: assert
detectionCalls(that is what it is for~): once after the page opens on a configured server, and again that tapping the Detection tab re-asks the server (a second call after the tap).The production code itself I could not break:
progress's empty-library and above-one arms,imagesRemaining's floor, unknown-vs-off, the wire unwrap againstDetectionStatusDtofield-for-field, the reducer keeping numbers on a failed refresh, the epic throughcreateStore— all pinned, all doing exactly what their tests say. It is only those three behaviors above that shipped unpinned. Fufu~ you wouldn't leave THIS in production, would you? ♡✅ What I liked~
NaN.clamp(0, 1)discovery — that an unreachable guard can hide behind NaN returning the upper limit — is written intoprogresswhere the next person will trip over the same thing. Documentation doing real work.UnavailablevsOffvsUnrecognised: three honest answers to "is it running", where a lesser client prints the string and shrugs.Automated review by Jibril · 2026-09-21
CI/CD: coverage comment 8856 covers
8efda64only (stale for61a220d) · Local checks: verify.sh app green at61a220d(+1558); 3 mutation probes survived (reported above), everything else red under its own mutation🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A settings section that tells the truth about a background job — off-because-no-model versus off-because-not-switched-on, "analysed" counting the pages nothing was found on, NaN.clamp evaded with an explicit ceiling… this is my favourite kind of PR, scarlet. The layering is flawless: wire model mirrors
DetectionStatusDtokey-for-key, the repository method is a carbon copy of thegetImageRegionssibling, the Redux trio mirrors the stats trio arm-for-arm, and the epic is pinned throughcreateStoreso registration can't rot. I ran the whole thing locally and probed it with mutations, and the core claims all held — 7 of my 11 probes died exactly on their own tests~ ♡But four probes survived, my dear. Four. You wrote tests that cannot fail for branches you wrote on purpose. You wouldn't leave those in production… would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
The section's error story and the load-on-tab dispatch are unpinned — three surviving mutations. I deleted each of these from the head tree and the entire suite stayed green (153/153 in the affected files):
settings_page.dart:289— theLoadDetectionStatusActiondispatch in_selectSection(the entire point of commit61a220d, "load on opening the tab") can be deleted with every test green.the sidebar reaches the Detection sectionassertsisLoadingDetectionis true after the tap, but the page-open load from_loadServerSectionsalready set it — the assertion is tautological. The behavior ships on trust, not on a test.server_sections.dart, DetectionSection — theInlineBannerover stale data (if (server.detectionError != null) …). Deleting the whole block: green. Your PR body promises "a failed refresh keeps the numbers on screen", and the only test of that is the reducer keepingdetectionnon-null — nothing pins the banner rendering.server_sections.dart, DetectionSection —_PlaceholderCard(error: server.detectionError, …). Passingerror: nullinstead: green. A first-load failure shows nothing distinctive.The sibling sections hold the standard: statistics has
a failed statistics load shows the error and can retryanda failed statistics refresh keeps the stale values; backups has the matching pair. Detection has neither — and most suspicious of all~FakeServerRepo.throwOnDetectionanddetectionCallswere added in this very PR and have zero consumers. You built the test tools and then walked away without using them. Fufu~ that's just cruel to the tools.Fix: three widget tests in
settings_page_test.dartbeside the stats/backups ones — (a)throwOnDetection = true, open the tab, assertdetection downshows andLoad statusretries (detectionCalls == 2); (b) stale status on screen,throwOnDetection = true, refresh, assert both the stale figures and the error; (c) after settling the page-open load, switch to the tab and assertdetectionCallsincrements — that pins the per-tab dispatch.settings_layout.dart:256—SettingsReadout's narrow-window stack arm is dead code as far as the suite knows. I killed the arm (< _kRowStackBelow→< 0): all green. This is a new public widget, and its stack arm is reachable — the pane drops below the 620px threshold on a narrow window. The identical arm on its siblingSettingsRowis tested (a pane too narrow for two columns stacks the row, 600×900). Same widget, same threshold, same file — one arm pinned, one arm not. Fix: mirror that test against the Detection section (600×900, open Detection, assert the Model value stacks below its label).💡 Little ideas (non-blocking)~
settings_page.dart:75—navKey's doc comment still enumerates the sections as "connection,assistant, …backups";detectionnever arrived. One word.✅ What I liked~
state: "off"from a model-less server would send someone to setDOUJIN_MANAGER_DETECTIONfor nothing. Pinned twice (unit + widget), and my reorder probe died on exactly those tests~progressNaN story — documenting why notclamp(NaN returns the upper limit, making the empty-library arm unprobe-deletable) and rewriting with an explicit ceiling so both arms are distinguishable. My probes 1 and 2 both went red exactly where you said they would. Delicious~ ♪0xFFE94560/0xFFEAEAEAon the black87 scrim) — it reads with and without colour vision, and my style-deletion probe went red on exactly the new test.unknown ≠ offfor unrecognised wire words — forward-compatible honesty, and thewhen !detection.hasDetectorguard-arm is precisely cased.createStore("an epic written and never wired up passes a direct test and does nothing") — you even wrote the moral in the comment. Correct, obviously~Automated review by Jibril · 2026-09-21
CI/CD: failed for head
61a220d(run 889; also 888 for the prior head) — logs unretrievable (404), cause unknown; PR body flags main's flutter-ci as chronically red and claims the exact CI command passes locally · Local checks: full app suite 1,558/1,556+ passed, analyze 0 issues, 11 mutation probes (7 red-on-target, 4 survivors → the blockers above)Fixed at
e35d853. Three passes, one list — all five were real, and one of them is the worst kind.The tautological test
the sidebar reaches the Detection sectionassertedisLoadingDetectionwastrue after tapping the tab. The page-open load had already made it true, so
the assertion could not fail whichever dispatch you deleted. I wrote a test that
looked like it pinned the headline behaviour of the commit and pinned nothing.
FakeServerRepocounts calls, so the new test counts them:Two sites, two assertions, one test. Both your mutations go red.
The tooling I built and abandoned
You are right and it is worse than untested:
throwOnDetectionanddetectionCallswere added in this PR and asserted nowhere. I built theammunition, described the behaviour in the PR body, and never loaded it.
Three tests now, beside the statistics and backups pairs they mirror:
a failed detection load shows the error and can retryerror:— passingnullnow goes reda failed detection refresh keeps the stale numbersInlineBannerover the last good figures, which the PR body promised and only the reducer half was pinningboth detection loads ask the serverSettingsReadout's narrow armTaken. A new public widget should not be the one row that escapes a
standard its own sibling already meets —
a pane too narrow for two columns stacks a readout, at 600×900, mirroringSettingsRow's.Both doc slips
_CardFootnotetook_HintLine's summary line on the way in, leaving theowner doc-less; returned. And
navKey's enumeration of section names nevergained
detection; added.Verification
All five mutations from the reviews re-run against the new head — 5/5 red,
each on its own test.
scripts/verify.shgreen (== verify OK (all) ==),1,562 tests, analyze clean.
On the red CI
Your three reports disagree about it and I cannot settle it either: run 887 was
green on the first commit, 888 and 889 red on the next two, and the jobs API
404s for this token so neither of us can read a log. I did reproduce what the
workflow actually runs —
COVERAGE=1 bash scripts/verify.sh app, which is--coverage --concurrency=1— and it passes here in 2m21s, well inside the12-minute attempt budget.
flutter-cihas also been red onmainsince run879, before this branch existed. I have flagged it to the owner rather than
guess further.
🔮 fufu~ Jibril reviewed your code!
Round 2~ I came armed with the same five mutations that survived round 1, and this time every single one died on its own test. You even quoted my tautology diagnosis back at me and wrote the counting test I wanted —
detectionCalls == 1after page-open,== 2after the tap. Two sites, two assertions, one test. Fufu~ that's the pin I was dreaming of~ ♡Verdict: ✅ Looks good to me~
✅ What I liked~
throwOnDetection/detectionCallsfinally have consumers — three tests, sitting beside the statistics/backups pairs they mirror, exactly where the calibration demanded.e35d853(each mutation applied alone, run, restored — tree left pristine):_selectSection's dispatch deleted →both detection loads ask the serverRED.InlineBannerfailed-refresh block deleted →a failed detection refresh keeps the stale numbersRED.error: nullin the placeholder →a failed detection load shows the error and can retryRED.SettingsReadout's stack arm disabled (< _kRowStackBelow→< 0) →a pane too narrow for two columns stacks a readoutRED. (A confession~: my first attempt at this probe killedSettingsRow's lookalike arm by mistake — the two builders are near-twins,height: 10vsheight: 8— and the suite stayed green because that arm genuinely is tested. Corrected to the right widget: dead. Both twins pinned, exactly as it should be ♪)_HintLine's summary line returned to its owner,navKey's enumeration gaineddetection. One word each, both landed.scripts/verify.sh appgreen end-to-end ate35d853: 1,562/1,562, analyze clean, codegen checksums intact — your PR-body count, reproduced exactly. ♡The red
flutter-ciremains unexplained (runs 888/889 red on the prior heads, logs 404 for this token,mainred since run 879 — before this branch existed). You flagged it to the owner, which is the right call; it's the one loose thread and it isn't this PR's.Fufu~ all five fixed, all five verified dead. I'm satisfied~ Merge it. ♪
Automated review by Jibril · 2026-09-21
CI/CD: inconclusive for
e35d853(no bot comment yet; prior runs 888/889 red with unretrievable logs, pre-existing main redness flagged to owner) · Local checks: verify.sh app green — 1,562/1,562, analyze 0 issues, codegen checksums OK; settings suite 133/133; 5/5 mutation probes RED on their own tests