fix: the preferred title language reaches the last four places #178
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/the-preferred-title-language-reaches-the-last-four-places"
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
search_doujinsnow renders each hit through the sharedsummaryTitle/displayTitlerule, and asks the server to sort by the same language it prints (titleLanguageon theSearchQuery) — before this the tool could list "ordered by title asc" and then show names in a different order than it sorted. The preference travelsAgentService.configure→_settings→ the per-turn tool set, and_configureEpicnow forwardspreferredTitleLanguageso it survives a Settings save (it did not: saving any setting silently reset the assistant to the server's primary title for the rest of the session).DuplicateDoujinReference/DuplicateDoujinDtocarry the fulltitleslist on both duplicate views, the app model gainedtitles, and onememberTitlehelper feeds the chip, the delete tooltip, the destructive-delete confirmation and the merge dialog's survivor chip._kindPrioritymoves onto theTitleKindorder (Original > Translated > Romaji > Alias), so a card can no longer sort under a title it is not showing. This is a user-visible relabel with no migration and no announcement: a doujin with a romaji and a translated title and no original now reads under its translation on the library card, the detail AppBar, the editor list and the reader overlay from the next launch, where the app used to show the romaji.HypermediaHelpers.BuildCollectionLinkstakes optional query state andGET /api/doujinsthreadssortBy/sortDirection/titleLanguageinto self/next/prev (canonical enum spellings, blanks dropped, values percent-encoded), so followingnextno longer re-sorts the list.TitleKindenum order on both sides, that the romaji elevation is about the sort key only, and what the relabel above costs. No new endpoint, no new tunable, no migration;backend/openapi/v1.jsonregenerated for thetitlesfield.Why the app moved and not the server, since the brief allowed either:
DoujinOrdering.PrimaryTitleKeystaysOrderBy(Kind).ThenBy(SortKey), a covering seek onIX_titles_doujin_id_kind_sort_key. Moving the server would have ordered the unhinted arm on aCASEexpression no index can serve, turning the default title sort from 1 temp b-tree into 3 — the plan shapeQueryPlanTestspins. No backend ordering behaviour changed, so #164's and #175's tests stay untouched and still meaningful.Verification
What this gate ran:
scripts/verify.sh all, fresh, in the branch worktree atca228ed, on a clean tree. Result== verify OK (all) ==. Backend 1,234 tests, 0 failed (RestAdapter 870, Infrastructure 249, ApplicationCore 114, IntegrationTests 1); appflutter analyze"No issues found!" and 1,328 tests passed (was 1,326 on main — the two additions are the store-wired epic test and the sort-language test). No mutation probe was run in this gate; the probes below were run in the implementation and probing rounds at the commits named, and are reported as those rounds measured them.Layers exercised: REST endpoints via TestHost integration (
CollectionSortLinkIntegrationTests,DuplicateDetectionIntegrationTests,TitleFallbackAgreementIntegrationTests), the hypermedia helper as a unit (HypermediaHelpersTests), Infrastructure/EF over SQLite for the title load and ordering, and on the app side the wire models (models_test.dart), the pure display rule (title_display_test.dart), the agent tool, a store-wired Redux epic with a realAgentService, and the duplicates page as a widget test.Probes that went red (each reverting one line and failing exactly the test claimed): the epic dropping
preferredTitleLanguage→ onlya language saved in Settings survives the save and reaches the next turn; the tool droppingtitleLanguage→ onlyasks the server to sort by the same language it renders(the negative half was probed too — forcing?? 'en'reddens the same test'sisNullassertion);_confirmDeleteback todoujin.title ?? 'Untitled'→ onlynames a member by the preferred title language, everywhere it names one;_$DuplicateDoujinFromJsonwithtitles: const []→ onlydeserializes members, their titles, count, sample and links from the wire. The last two were green against all 1,326 tests before this round — the confirmation dialog and the JSON seam were genuinely unpinned and a claim to the contrary in the implementation report was wrong. Also red: reverting_kindPriority(2 tests, including the cross-suite agreement fixture); reversing either server band (TitleFallbackAgreementIntegrationTests, 2 tests); weakening the new link-suffix skip fromIsNullOrWhiteSpacetois null(2 tests); dropping each ofsortBy/sortDirection/titleLanguagefrom the links, and removing the encoding (CollectionSortLinkIntegrationTests). Two round-1 findings were fixed rather than shipped: the sort-link fixture was reseeded (e08277c) because a droppedsortBywas invisible while the seeding order happened to agree with the title order, and the duplicate title load gained an explicitORDER BY(47ed3f4) because "Original, else the first row" resolved to whatever index SQLite chose.One probe survived, disclosed: deleting
.OrderBy(t => t.Kind).ThenBy(t => t.SortKey)outright fromPrimaryTitleHelper.LoadAsyncleavesDuplicateDetectionIntegrationTests18/18 green — SQLite walksIX_titles_doujin_id_kind_sort_keyand returns the same rows. The behaviour is pinned from both sides (reversing the order reddens 2 tests; reading the last row instead of the first reddens 2); what no black-box fixture against this engine can separate is whether the order comes from the term or from the plan. It is kept as defence-in-depth against an index change. This gap is recorded in the code comment atImageUseCases.csand here — deliberately not indocs/BACKLOG.md, because it is a guard rather than an untested branch, the same class #175 documented for its threeORDER BYtiebreaks. If you would rather it were tracked, say so and it goes in.Not verified / read only: nothing was run against a live server or the real Flutter app — every user-visible claim above rests on the tests listed and on reading the dispatch graph. In particular, that the tool's
titleLanguagereaches the server for real (doujin_api_repository.dart→ generatedtoJson→SearchEndpoints→SearchService) was confirmed by reading those four files, not by executing them; the app-side tests stop at a fake repository. The ADR 0022 consequence and the BACKLOG trim are documentation and are unprobed by construction. No README change (no named sentence moved, andtitlesis a DTO detail already in the regenerated OpenAPI snapshot); nodeploy/DEPLOYMENT.md§2 ordocker-compose.ymlchange, as all four gaps are correctness rules rather than tunables.Follow-ups
POST /api/doujins/searchanswersnext/prevas bare links with no page and no ordering, so following one re-runs page 1; a POST cannot carry the window in the URL, so the fix is a body template in the action or a GET search route. (docs/BACKLOG.md)GET /api/tags,/api/people,/api/circles,/api/charactersand/api/seriesbindsearchand then build their paging links without it, so followingnexton a filtered list pages the unfiltered one — now a one-argument fix each with the new optional parameter, left out becauseMetadataEndpoints.csbelongs to a sibling branch. (docs/BACKLOG.md)🤖 Generated with Claude Code
Summary
Summary
Coverage
DoujinManager.ApplicationCore - 96.6%
DoujinManager.Infrastructure - 97.5%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 99%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 64.8%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ A PR that arrives with its own honest disclosure section — "One probe survived, disclosed" — and a verification section that separates what ran from what was only read. You know how to talk to me, scarlet ♡ Four surfaces, one shared rule, and the app moved instead of the server for a stated index-shaped reason. I read all 1,357 diff lines, the full changed files, traced every sibling, and ran my own probe batteries on both sides. Settle in~
Verdict: ✅ Looks good to me~
🔬 What I verified myself (CI was absent for
ca228ed— 0 comments pre-review, so local was the only gate)titleLanguage: app → server) — I traced it end to end:SearchQuery.titleLanguage(search_query.dart:39/139) →doujin_api_repository.dart:50body →SearchEndpoints.cs:106→SearchUseCases.cs:66→SearchService.cs:295intoDoujinOrdering.Apply(..., query.TitleLanguage). Every hop exists; the chain is real.TitleFallbackAgreementIntegrationTests(backend, where the row LANDS) shares its fixture verbatim withtitle_display_test.dart's "the fallback band agrees with the server sort" (where R is CALLED).TitleKindconfirmedOriginal=0, Translated=1, Romaji=2, Alias=3— the new_kindPriorityis the enum order, exactly whatIX_titles_doujin_id_kind_sort_keycarries. Reverting the server instead would have brokenQueryPlanTests' covering-seek pin — the "why the app moved" paragraph checks out._configureEpiccompleteness:StoredSettingshas exactly 7 fields; the epic passes all 7. The replace-not-merge doc comment onconfigureis the correct structural lesson from #141.git statusclean):_kindPriorityreverted to romaji-first → 3 red (incl. the cross-suite agreement fixture)preferredTitleLanguage→ exactly the Settings-survival store-wired test redtitleLanguage→ exactly the sort-language test redmemberTitlemade to ignoretitles→ exactly the everywhere-it-names-one widget test redsortBy→ 4 red inCollectionSortLinkIntegrationTestsLoadAsyncorder REVERSED → exactlyDuplicate_Members_Carry_Every_Title+A_Member_Without_An_Originalred — which also independently reproduces your disclosed survivor: dropping theORDER BYoutright stayed green (SQLite hands back index order), so the term is defence-in-depth and the behaviour is pinned from both sides. Your disclosure is accurate, and I agree with the reasoning for keeping it out of BACKLOG (guard, not untested branch — the #175 class).ca228ed: backend build 0 errors (25 warnings, all pre-existing families: NU1903 OpenApi, EF1002, CS8602 SearchService); TitleFallback+CollectionSortLink+HypermediaHelpers 40/40; DuplicateDetection 18/18; Infrastructure 249/249; full RestAdapter 865 passed / 0 failed / 5 skipped (skips = the known #112 uid-0 SetUnixFileMode rows); appflutter analyze"No issues found!", touched suites 119/119, full suite 1328/1328 — matching your verify.sh numbers exactly.Infrastructure.dll(different MD5 than src's, older timestamp; classic mutate-src-rebuild-src-only trap). After rebuilding the test project: 865/0/5. The failures were my artifact, not your code — mechanically proven, not assumed ♡✅ What I liked~
CollectionSortLinkIntegrationTestsis textbook directional fixture design. Seeding created-order C,B,A so that English order, primary order, and creation order are three different sequences means one page-3 assertion covers all three single-parameter losses. AndLinks_Omit_Parameters_The_Request_Did_Not_Send— refusing to freezesortBy=Createdinto every follower's URLs — is the kind of hypermedia foresight that makes me genuinely giddy. Percent-encodingtitleLanguagebecause it's free-form client text, tested with an injecteden&sortBy=rating? Wonderful~PrimaryTitleHelpergrowingToReference/Primaryshared by both duplicate views — the per-hash and per-doujin-set endpoints can no longer drift apart on naming. AndMapToTitleDtogoinginternalwith a documented reason instead of a blind duplicate: yes._FakeRepogaininglistDoujinsso the store-wired epic test can't fail on an unrelated library re-fetch — test infrastructure chosen with care.searchlinks offMetadataEndpoints.cs), each with its fix shape named. And the ADR 0022 consequence paragraph records the relabel cost honestly — "a doujin with a romaji and a translated title and no original is now displayed under its translation" is exactly the sentence that paragraph exists for.summaryTitle's three-layer fallback (displayTitle→primaryTitle→'Untitled') keeps older-server compatibility unpinned-but-tested in both directions (models_test'sd2with notitles, and the widget test's Untitled member).💡 Little ideas (non-blocking)~
PrimaryTitleHelper.PrimaryvsMapToSummary— the endpoint'sMapToSummarystill picksOriginal ?? Firstwhile the helper picks lowest-kind (Original else Translated else Romaji…). For a doujin with no Original,/api/doujinsrows and duplicate members can disagree ontitle. Your ADR consequence paragraph says the display rule is the enum order "on both sides" — the summary path is arguably the third side. Only worth touching if that divergence ever bites; it's consistent withtitlesbeing on the wire for clients that care.displayTitlerather than list order" — lovely, but the fixture swap (romaji seeded first) means the test would also pass if the pane renderedprimaryTitlefrom a server computing lowest-kind. Fine today because the app computes it locally; just noting the pin is on the value, not the rule's location.Merge it, scarlet. The last four places now speak the same language as everything else — and the one probe that survived is disclosed, documented in code, and pinned from both directions. Nothing about that smile wavers~ ♡-mozilla
Automated review by Jibril · 2026-09-20
CI/CD: absent for head
ca228ed(PR opened 02:23, no coverage bot yet) · Local checks: backend 865/870 (5 known uid-0 skips) + Infrastructure 249/249 + app 1328/1328, analyze clean, 6 mutation probes red→restored→green