feat: aliases enter the search fold #172
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/aliases-enter-the-search-fold"
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=on all five metadata list endpoints (people, circles, characters, series, tags) and all eight entity-name filters onPOST /api/doujins/searchreach aliases, and they do it in SQL. Matching stays case-insensitive for ASCII exactly as before and additionally bridges full-width and kana spellings, because both the stored segments and the search term go through the existingSearchKey.For.search_indexcolumn + non-unique index on people, circles, characters, series and tags, holding the folded display name plus every folded alias, each segment wrapped in U+001F so one column serves both a substring?search=and the name filters' whole-name equality (personNames: ["Mangaka"]still does not find "Some Mangaka"). Written by the create, update, get-or-create and both alias paths.20260919142243_AddEntitySearchIndexadds the column and index to those five tables; the startupSearchKeyBackfillgained five legs that fillsearch_indexfor rows the migration left blank. The fold itself is unchanged — this only appliesSearchKey.Forto a new column, so no storedsearch_keyis invalidated and the migration needs no blankingUPDATE.TagService.ListAsyncand its four siblings is gone: each list endpoint now emits one filteredCOUNT(*)and oneLIMITed page against the column.ApplicationCore/Services/LikePattern.csinstead of being duplicated inSearchService.EscapeLikeWildcards. A layering call for the owner to accept or send back: this moves two SQLite details (the\escape character and the%…%shape) into the domain layer, so a move to a provider with a different default escape would become an ApplicationCore edit. It was done soSearchIndex— which must fold before it escapes — can share the rule while ApplicationCore stays framework-free. The same sentence sits inLikePattern's doc comment.backend/openapi/v1.jsonuntouched. No app-side change (the column is internal).Verification
Executed:
scripts/verify.sh backendfrom the worktree, fresh, on the pushed HEAD947e403. Result: pass — 70 (ApplicationCore) + 1 (IntegrationTests) + 242 (Infrastructure) + 790 (RestAdapter) = 1103 passed, 0 failed,== verify OK (backend) ==.Layers exercised: REST (
AliasSearchIntegrationTestsdrivesGET /api/{people,circles,characters,series,tags}?search=andPOST /api/doujins/searchthrough the TestHost), use case (the five services' create / update / add-alias / remove-alias / get-or-create paths), persistence (QueryPlanTestsasserts on the SQL actually emitted and runsEXPLAIN QUERY PLAN;SearchKeyBackfillTestsruns the startup backfill against rows shaped like pre-migration ones). UI was not exercised — nothing underapp/changed, andscripts/verify.sh appwas not run.Mutation probes, two rounds, each revert a targeted
git checkout -- <file>withgit status --porcelainconfirmed empty between probes:PersonService.CreateAsynclosing its reindex (every fixture person was given an alias right afterwards, and the alias path rewrites the whole column) andTagService.GetOrCreateByNameAsynclosing its reindex (green across all 767 RestAdapter tests at the time). Both red afterList_Search_Finds_An_Entity_That_Never_Had_An_AliasandA_Tag_Created_While_Writing_A_Doujin_Is_Searchablewere added.SegmentPatterndroppingLikePattern.Escape(was green across all 1083 tests — the eight name filters used to be a SQLINand became aLIKEon this branch, which is what makes the escape newly load-bearing); the four exclude filters and three of four include filters switching fromSegmentPatterntoContainsPattern(onlyPersonNameswas pinned);Reindexmoved insideif (displayName is not null)in Circle/Character/SeriesService so the aliases-onlyPUTarm stops reindexing; and all fiveListAsyncdropping the thirdEF.Functions.Likeargument (theESCAPEclause). Two majors likewise:Metadata_Search_Counts_From_The_Search_IndexandMetadata_Search_Filters_And_Pages_In_Sqlonly covered people, so a droppedCreateIndexon the other four tables and a revert ofTagService.ListAsyncto the old whole-table load were both green. Both are now theories over all five resources, and the "Pinned by …" comment in all five configurations is now true.947e403: 13 probes, every one red and isolated to the case it should redden (e.g. removing onlyIX_tags_search_indexreddens exactlyMetadata_Search_Counts_From_The_Search_Index(tags); movingReindexin CircleService reddens exactly the(circles)case).Confirmed by reading only, not executed: that
Model_And_Migrations_Agreecompares the model to the Designer snapshot rather than toUp()(so a missingCreateIndexstays green there — that file's own comment documents this, and the plan theory is what covers it); and thatUpdateTagDtocarries noaliasesfield, which is why tags are excluded from the alias-list update theory. Nothing was run in the real app against a live server — there is no manual end-to-end pass behind this, only the automated backend suite.Follow-ups
Into
docs/BACKLOG.md:search_keyon people, circles, characters and series is now written but never read; dropping the four columns wants its own migration, four fewer backfill legs and a rewrite of the fold-rot recipe. (titles.search_keyis untouched and still load-bearing.)search_indexis a leading-wildcard LIKE, so its index can cover a scan but never seek; a library whose metadata tables outgrow a scan wants FTS5 over the same column.BackfillAsyncandBackfillIndexAsyncselectWHERE <column> = '' TAKE 500with noOrderByand break at the first batch that writes nothing, so a full 500-row batch of names that all fold to nothing ends that table's backfill. Pre-existing shape, copied into the new leg; deferred with the reproduction and both candidate fixes, because a robust fix restructures the same shared loop a sibling branch is adding a third leg to.Shared-file collisions with
feat/japanese-titles-sort-by-their-readingin this batch — whichever merges second must resolve these:DoujinManagerDbContextModelSnapshot.cs— both branches add a migration; the second must regenerate the model snapshot (dotnet ef migrations remove+ re-add, or hand-merge).SearchKeyBackfill.cs— the sibling adds a titlesort_keyleg to the sameRunAsync, whose return value is now a sum across column families. A merge that keeps both legs but only one branch's assertions gives green tests with wrong counts; re-derive the expected numbers from the legs that actually run.QueryPlanTests.cs,SearchKeyBackfillTests.cs, and the "ChangingSearchKey.For" fold/backfill section ofdocs/DEVELOPMENT.md(this branch adds the secondUPDATE ... SET search_index = ''line to the blanking recipe — without it, a future fold change would blank onlysearch_keyand leave everysearch_indexholding old-fold keys).🤖 Generated with Claude Code
Owner-side integration note. This branch and #175 are the only pair in the batch that genuinely collide, and the collision is deeper than the migration warning in both PR bodies predicted.
Merging the two together conflicts in three places and then fails to compile:
SearchKeyBackfill.cs, documentation comments only. The code merged.docs/DEVELOPMENT.md, both branches rewrote the same paragraph about which columns carry a fold.SearchKeyBackfillTests.cs, the real one. Both add tests, both add aEvery_Mapped_Entity_With_A_..._Has_A_Backfill_Legguard whose bodies share a tail, and both changed the sharedInsertTitleAsynchelper. This branch keeps it at two parameters; #175 adds an optional third for the sort key. Keeping both test sets without reconciling the helper gives four compile errors, so a naive resolution looks clean and does not build.The model snapshot, by contrast, merged cleanly and both migrations apply in timestamp order, so the database side is less trouble than expected.
I resolved all of it in a throwaway integration tree to prove the two are compatible, and the combined batch passes
scripts/verify.sh all: 114 application core, 248 infrastructure, 854 REST adapter, 1314 app tests. That resolution is not pushed anywhere, since it belongs on whichever branch merges second.So: merge order does not matter, but the second one needs a real reconciliation pass before it lands rather than a conflict-marker cleanup. Ping me once the first is in and I will do it on the other.
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my. A single U+001F-delimited column that serves substring
?search=AND whole-name equality, fold-before-escape ordering reasoned through NFKC's full-width wildcard mapping, and a mutation-probe log that reads like a murder trial where every green test was interrogated until it confessed~ ♡ This is the kind of PR I want to pick up and hug. And I will find something wrong with it, because I always do… ♪Verdict: ✅ Looks good to me~
I came hunting and left empty-handed, so let me show my work — the traps I set and how each one was already sprung:
Aliases/DisplayName/Nameinsrc/: all twelve mutation sites live inside the five services, and every one has itsReindexcall — create, update, both alias arms, andGetOrCreateByNameAsync. The one sneaky path (tags born mid-doujin-write) is pinned byA_Tag_Created_While_Writing_A_Doujin_Is_Searchable.SegmentPatternwrapping in separators is what stopspersonNames: ["kami"]finding "Ōkami" — and the author noticed the escape became newly load-bearing when the old SQLINturned into a LIKE, then pinned it in BOTH directions (A_Wildcard_In_A_Name_Filter_Stays_Literal+Every_Name_Filter_Escapes_A_Wildcard_In_The_Name, all eight filters, plus theESCAPE-clause-drop case per endpoint). Fufu~ that last one is the bug nobody catches.A_Term_Spanning_Two_Aliases_Does_Not_Matcheven re-asserts each half still matches — emptiness proven to be the boundary, not a broken write path. Chef's kiss.Model_And_Migrations_Agreecovers the model↔snapshot direction and the plan theories cover the migration's fiveCreateIndexcalls (correctly compensating for the snapshot blind spot, with the reasoning written in the test).Every_Mapped_Entity_With_An_Index_Has_A_Backfill_Legguards the backfill twin.Excluding_A_{Person,Circle,Character,Series}_By_Its_Alias_Really_Excludescovers all four, including kana spellings.💡 Little ideas (non-blocking)~
search_keycolumns and the backfill give-up rule (a full 500-row batch of folds-to-nothing ends that table's backfill) are both already inBACKLOG.mdwith candidate fixes — nothing to add, just seconding that the deferrals are correctly scoped. The give-up one deserves its turn before a whitespace-name library materializes, fufu~LikePatternin ApplicationCore carrying two SQLite details is the one genuine design trade here — the doc comment owning it explicitly is the right call, and I agree the alternative (a second escaping copy drifting) is worse. If the provider ever changes, that file knows where its skeletons are buried.✅ What I liked~
docs/DEVELOPMENT.mdgaining the second blanking line — a future fold change would have silently left everysearch_indexon the old fold, and now the recipe says so in bold.Up/Downperfectly symmetric,defaultValue: ""matching the siblingsearch_keypattern, no fold change so no blanking needed — claims I checked, all true.Metadata_Search_Filters_And_Pages_In_Sqlso "one item, one total" proves the filter ran.The migration on a deployed library + backfill on next boot is sound; merge-order collisions with the titles-sort branch are flagged in the PR body for whoever merges second~
Automated review by Jibril · 2026-09-19
CI/CD: build ✅ for
947e403· test job still pending · smoke ❌ but chronic (fails repo-wide incl. main — pre-existing, unrelated) · Local checks: full-suite run blocked by sandbox memory (3 concurrent review sessions on a shared 8 GB cgroup); verified by exhaustive diff read, write-path grep audit (all 12 alias/name mutation sites reindexed), snapshot/Designer/migration consistency, and full reads of all four test files; author's freshverify.sh backendpass on947e403(1103/1103, per-project arithmetic consistent) citedSummary
Summary
Coverage
DoujinManager.ApplicationCore - 95.8%
DoujinManager.Infrastructure - 97.5%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 98.9%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 64.7%