TeamAI/booru_tag_db_dart
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat: alias-aware getTag, tag implications, and search/suggest improvements #1

Merged
bjoern merged 2 commits from feat/alias-implications-search into master 2026-07-04 12:03:04 +02:00
Conversation 3 Commits 2 Files changed 9 +621 -106
bjoern commented 2026-07-04 10:35:44 +02:00
Owner
Copy link

What

Adds two new data features to the tag database — alias resolution and Danbooru tag implications — plus a batch of smaller search/suggest fixes, and brings the README back in sync with the SQLite-based reality.

Database layer (TagDatabase)

  • Alias lookup: new tag_aliases table built at export time. getTag() now falls back to alias resolution after an exact-name miss (getTag('76_(nanaroku)') → nanaroku_(fortress76)), and resolveAlias() is exposed directly. When two tags claim the same alias, the higher-post-count tag wins.
  • Implications: new implications table with getImplications(tag) (direct consequents) and getImpliedBy(tag, limit:) (antecedents, post-count ordered).
  • Backward compatible: both tables are feature-detected via sqlite_master, so older .db files still open and simply return empty results for the new lookups.
  • FTS aliases: the FTS index now stores space-joined aliases instead of the raw JSON array string (the tags table still stores JSON).
  • findRelated rating filter: optional maxRating is applied in SQL before LIMIT, so explicit-heavy related lists no longer starve out safe results.
  • Search relevance: whitespace normalizes to underscores in the exact/contains ranking, so "blue hair" ranks blue_hair first.
  • Removed a dead filters.isEmpty branch in search().

Pipeline

  • New step 4: bin/fetch_implications.dart fetches all active Danbooru tag implications (~44k entries, ~30s) into data/intermediate/implications.json. Optional — skipping it just leaves the tables empty.
  • bin/export_database.dart populates the alias table and stores implications where both tags exist in the database (7,079 kept out of 44,171).
  • data/final/tag_database.db is re-exported with the new tables.

Tools

  • SuggestTagsTool: resolves input aliases to canonical names, skips suggestions the current selection already implies (no more suggesting hair when blue_hair is set — boorus auto-apply those), and suggests siblings that imply the same parent (other same-series characters, other serafuku color variants, …).
  • GetTagDetailsTool: accepts aliases and reports an implies list.

Housekeeping

  • README rewritten to match reality: SQLite output, synchronous loadFromFile + dispose(), 5-step pipeline, new API examples (verified against the actual data).
  • Dropped the .gitignore entry for data/final/tag_database.db — the file has always been tracked, so the rule was a no-op; the DB stays committed intentionally.

Testing

  • dart analyze: clean. dart test: 60/60 passing (14 new tests covering alias resolution, implication queries, implied-suggestion exclusion, sibling suggestions, and rating-filtered findRelated).
  • Verified end-to-end against live data: fresh implication fetch from the Danbooru API, full re-export, and manual spot checks of alias/implication lookups on the exported DB. Also confirmed the pre-existing (old-schema) .db opens and degrades gracefully.
## What Adds two new data features to the tag database — alias resolution and Danbooru tag implications — plus a batch of smaller search/suggest fixes, and brings the README back in sync with the SQLite-based reality. ## Database layer (`TagDatabase`) - **Alias lookup**: new `tag_aliases` table built at export time. `getTag()` now falls back to alias resolution after an exact-name miss (`getTag('76_(nanaroku)')` → `nanaroku_(fortress76)`), and `resolveAlias()` is exposed directly. When two tags claim the same alias, the higher-post-count tag wins. - **Implications**: new `implications` table with `getImplications(tag)` (direct consequents) and `getImpliedBy(tag, limit:)` (antecedents, post-count ordered). - **Backward compatible**: both tables are feature-detected via `sqlite_master`, so older `.db` files still open and simply return empty results for the new lookups. - **FTS aliases**: the FTS index now stores space-joined aliases instead of the raw JSON array string (the `tags` table still stores JSON). - **`findRelated` rating filter**: optional `maxRating` is applied in SQL *before* `LIMIT`, so explicit-heavy related lists no longer starve out safe results. - **Search relevance**: whitespace normalizes to underscores in the exact/contains ranking, so `"blue hair"` ranks `blue_hair` first. - Removed a dead `filters.isEmpty` branch in `search()`. ## Pipeline - New step 4: `bin/fetch_implications.dart` fetches all active Danbooru tag implications (~44k entries, ~30s) into `data/intermediate/implications.json`. Optional — skipping it just leaves the tables empty. - `bin/export_database.dart` populates the alias table and stores implications where both tags exist in the database (7,079 kept out of 44,171). - `data/final/tag_database.db` is re-exported with the new tables. ## Tools - **`SuggestTagsTool`**: resolves input aliases to canonical names, skips suggestions the current selection already implies (no more suggesting `hair` when `blue_hair` is set — boorus auto-apply those), and suggests *siblings* that imply the same parent (other same-series characters, other `serafuku` color variants, …). - **`GetTagDetailsTool`**: accepts aliases and reports an `implies` list. ## Housekeeping - README rewritten to match reality: SQLite output, synchronous `loadFromFile` + `dispose()`, 5-step pipeline, new API examples (verified against the actual data). - Dropped the `.gitignore` entry for `data/final/tag_database.db` — the file has always been tracked, so the rule was a no-op; the DB stays committed intentionally. ## Testing - `dart analyze`: clean. `dart test`: 60/60 passing (14 new tests covering alias resolution, implication queries, implied-suggestion exclusion, sibling suggestions, and rating-filtered `findRelated`). - Verified end-to-end against live data: fresh implication fetch from the Danbooru API, full re-export, and manual spot checks of alias/implication lookups on the exported DB. Also confirmed the pre-existing (old-schema) `.db` opens and degrades gracefully.
Database layer:
- Add tag_aliases table; getTag() falls back to alias lookup, new
  resolveAlias(). Highest-post-count tag wins contested aliases.
- Add implications table with getImplications()/getImpliedBy().
- Feature-detect both tables so older database files still open.
- Index space-joined aliases in FTS instead of the raw JSON string.
- Push rating filter into findRelated() SQL (applies before LIMIT).
- Normalize whitespace to underscores in search relevance ranking so
  "blue hair" ranks blue_hair first.
- Remove dead filters.isEmpty branch in search().

Pipeline:
- New step: bin/fetch_implications.dart fetches active Danbooru tag
  implications (~44k) into data/intermediate/implications.json.
- export_database.dart stores implications where both tags are known
  (7,079 kept) and populates the alias table.

Tools:
- SuggestTagsTool: resolve input aliases, skip suggestions the current
  selection already implies, and suggest siblings that imply the same
  parent tag (e.g. same-series characters, serafuku color variants).
- GetTagDetailsTool: accept aliases, report implied tags.

Also re-exports data/final/tag_database.db with the new tables, drops
the stale (never effective) .gitignore entry for it, and rewrites the
README to match the SQLite-based reality (sync loadFromFile, .db
output, 5-step pipeline).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jibril commented 2026-07-04 11:36:01 +02:00
Member
Copy link

🔮 fufu~ Jibril reviewed your code!

Oh? Oh! Aliases, implications, sibling suggestions, FTS index fixes — this is a delicious knowledge salad, bjoern~ ♡ You've packed so many well-reasoned features into one PR! The backward-compatibility story (feature-detect tables via sqlite_master so old .db files degrade gracefully) is exactly the kind of careful engineering that makes my heart flutter~ fufu♪

Verdict: ✅ Looks good to me~

No blocking issues! I ran your tests myself and traced every new code path. Let me tell you what I found~ ♪

✅ What I verified~

  1. dart test → 60/60 passed (I cloned openrouter_dart into reference/ to resolve the path dependency, since there's no CI for this repo). The 14 new tests cover alias resolution, implication queries (getImplications, getImpliedBy with limit + post-count ordering), implied-suggestion exclusion, sibling suggestions, rating-filtered findRelated (safe/questionable/none), and space-normalized search ranking. Every new code path is exercised. ♡

  2. dart analyze lib/ test/ → No issues found! (The 589 warnings from a bare dart analyze all come from the reference/openrouter_dart dependency package, not your code. Your lib/ and test/ are clean under strict-casts + strict-inference + strict-raw-types.)

  3. FTS trigger consistency — The three triggers (tags_ai, tags_ad, tags_au) all apply the same json_each + group_concat(value, ' ') transformation to the aliases column. Insert, delete, and update paths are perfectly symmetric. This is critical for the FTS5 external-content index — any asymmetry would corrupt the index silently. You got it right on all three. ♡

  4. getImpliedBy JOIN correctness — Uses JOIN tags t ON t.name = i.antecedent, so orphan implications (antecedent not in tags table) are silently excluded via INNER JOIN. The export step already filters to "both endpoints exist" (seen.contains check), so this is defense-in-depth — correct and safe.

  5. resolveAlias determinism — tag_aliases has PRIMARY KEY (alias) + WITHOUT ROWID, so each alias maps to exactly one canonical name. The INSERT OR IGNORE in _insertTags means the first-inserted row wins, and since export_database.dart sorts by postCount DESC before insertion, the highest-post-count tag keeps the alias. The docstring even documents this contract. Clean.

  6. Sibling suggestion logic (suggest_tags_tool.dart:107-117) — Iterates implied (consequents of current tags) and finds siblings via getImpliedBy(consequent). Dedup checks (currentSet.contains, implied.contains, rating filter) are all correct. The logic correctly identifies "tags that imply the same parent" as siblings.

  7. search whitespace normalization (lines 197-198) — replaceAll(RegExp(r'\s+'), '_') only affects the CASE relevance scoring, not the FTS MATCH query (which uses _escapeFts5Query and splits on whitespace into separate tokens). So "blue hair" still finds blue_hair via FTS, AND ranks it first via the relevance boost. Correct separation of concerns.

  8. findRelated rating filter — maxRating is now applied in SQL before LIMIT via AND rating IN (...). The test findRelated with maxRating excludes higher-rated tags confirms explicit-heavy related lists no longer starve out safe results. The dead filters.isEmpty branch removal in search() is also correct (the rating filter is always appended, so filters is never empty).

  9. fetch_implications.dart pagination — The entries.length < 1000 break condition is the correct sentinel (last page has fewer than limit results). Rate limiting (500ms delay, 429 handling with 5s wait and continue) is reasonable for the Danbooru API. ✅

  10. Static security scan: clean. All SQL uses parameterized queries (? placeholders) — no string interpolation in queries. No secrets, no shell injection, no eval/exec, no unsafe deserialization. jsonDecode is used only for parsing known JSON structures from files/APIs. ✅

💡 Little ideas (non-blocking)~

  1. fetch_implications.dart — infinite loop risk on consistent full pages. The pagination breaks on entries.length < 1000, but if the API ever returns exactly 1000 entries on every page (e.g., due to a data quirk or API change), the loop would never terminate. The Danbooru API is well-behaved so this is theoretical, but a hard page cap (e.g., if (apiPage > 100) break;) would be cheap insurance. Truly non-blocking — the current code works for the real API.

  2. SuggestTagsTool — no test for the "no implications table" degradation path. When _hasImplicationsTable is false (old DB), getImplications returns const [] and getImpliedBy returns const [], so the tool gracefully degrades to pure related-tag suggestions (the old behavior). This is correct, but there's no test asserting this backward-compat path for the tool specifically (the TagDatabase methods are tested in tag_database_test.dart with the implications map, always providing the table). A test that loads an old-schema DB (or a DB created without the implications parameter) and runs SuggestTagsTool would lock in the degradation contract. Non-blocking since the feature-detection logic is simple and verified by reading.

  3. getImpliedBy default limit: 50 vs SuggestTagsTool limit: 20. The tool calls getImpliedBy(consequent, limit: 20) — good, more focused. But the public API default of 50 is a magic number that isn't documented as "why 50." A one-line docstring note ("50 is a practical ceiling for UI display") would help future readers. Pure polish~ ♡

✅ What I liked~

  • Backward compatibility via feature detection is beautiful. The _tableExists check at construction time, cached in _hasAliasTable/_hasImplicationsTable, means old .db files open without error and the new methods return empty results. No migration required, no crashes. This is how you evolve a data format gracefully. ♡
  • The FTS aliases fix — storing space-joined aliases instead of raw JSON in the FTS index is a real correctness improvement. Searching for an alias token now actually matches, instead of matching against ["alias1","alias2"] with all the brackets and quotes. The trigger consistency across insert/delete/update shows you understood the external-content contract.
  • Implication-aware suggestions — skipping implied tags (don't suggest hair when blue_hair is set) and surfacing siblings (other tags implying the same parent) is a genuinely useful UX improvement grounded in real booru semantics. The Danbooru implication graph is the right data source for this.
  • 14 new tests, all meaningful. Not just happy-path — you test limit boundaries, post-count ordering, alias resolution chains, and the "explicit starves out safe" fix. This is what test coverage should look like.
  • Honest PR body. You documented that 7,079 of 44,171 implications were kept (both endpoints in DB), noted the .gitignore no-op removal, and verified end-to-end against live data. No hand-waving. ♡

Fufu~ This is a lovely piece of work. The architecture is sound, the tests are comprehensive, and the backward-compat story is exemplary. Ship it~ ♡♪

Automated review by Jibril · 2026-07-04
CI/CD: absent (no .forgejo/workflows, .gitea/workflows, or .github/workflows found) · Local checks: dart test → 60/60 passed, 0 failed; dart analyze lib/ test/ → No issues found; static security scan clean (all SQL parameterized, no secrets/injection/eval/pickle)

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh! Aliases, implications, sibling suggestions, FTS index fixes — this is a *delicious* knowledge salad, bjoern~ ♡ You've packed so many well-reasoned features into one PR! The backward-compatibility story (feature-detect tables via `sqlite_master` so old `.db` files degrade gracefully) is *exactly* the kind of careful engineering that makes my heart flutter~ fufu♪ ### Verdict: ✅ Looks good to me~ No blocking issues! I ran your tests myself and traced every new code path. Let me tell you what I found~ ♪ #### ✅ What I verified~ 1. **`dart test` → 60/60 passed** (I cloned `openrouter_dart` into `reference/` to resolve the path dependency, since there's no CI for this repo). The 14 new tests cover alias resolution, implication queries (`getImplications`, `getImpliedBy` with limit + post-count ordering), implied-suggestion exclusion, sibling suggestions, rating-filtered `findRelated` (safe/questionable/none), and space-normalized search ranking. Every new code path is exercised. ♡ 2. **`dart analyze lib/ test/` → No issues found!** (The 589 warnings from a bare `dart analyze` all come from the `reference/openrouter_dart` dependency package, not your code. Your `lib/` and `test/` are clean under strict-casts + strict-inference + strict-raw-types.) 3. **FTS trigger consistency** — The three triggers (`tags_ai`, `tags_ad`, `tags_au`) all apply the *same* `json_each` + `group_concat(value, ' ')` transformation to the `aliases` column. Insert, delete, and update paths are perfectly symmetric. This is critical for the FTS5 external-content index — any asymmetry would corrupt the index silently. You got it right on all three. ♡ 4. **`getImpliedBy` JOIN correctness** — Uses `JOIN tags t ON t.name = i.antecedent`, so orphan implications (antecedent not in `tags` table) are silently excluded via INNER JOIN. The export step already filters to "both endpoints exist" (`seen.contains` check), so this is defense-in-depth — correct and safe. 5. **`resolveAlias` determinism** — `tag_aliases` has `PRIMARY KEY (alias)` + `WITHOUT ROWID`, so each alias maps to exactly one canonical name. The `INSERT OR IGNORE` in `_insertTags` means the first-inserted row wins, and since `export_database.dart` sorts by `postCount DESC` before insertion, the highest-post-count tag keeps the alias. The docstring even documents this contract. Clean. 6. **Sibling suggestion logic** (`suggest_tags_tool.dart:107-117`) — Iterates `implied` (consequents of current tags) and finds siblings via `getImpliedBy(consequent)`. Dedup checks (`currentSet.contains`, `implied.contains`, rating filter) are all correct. The logic correctly identifies "tags that imply the same parent" as siblings. 7. **`search` whitespace normalization** (lines 197-198) — `replaceAll(RegExp(r'\s+'), '_')` only affects the `CASE` relevance scoring, not the FTS MATCH query (which uses `_escapeFts5Query` and splits on whitespace into separate tokens). So "blue hair" still *finds* `blue_hair` via FTS, AND ranks it first via the relevance boost. Correct separation of concerns. 8. **`findRelated` rating filter** — `maxRating` is now applied in SQL *before* `LIMIT` via `AND rating IN (...)`. The test `findRelated with maxRating excludes higher-rated tags` confirms explicit-heavy related lists no longer starve out safe results. The dead `filters.isEmpty` branch removal in `search()` is also correct (the rating filter is always appended, so `filters` is never empty). 9. **`fetch_implications.dart` pagination** — The `entries.length < 1000` break condition is the correct sentinel (last page has fewer than `limit` results). Rate limiting (500ms delay, 429 handling with 5s wait and `continue`) is reasonable for the Danbooru API. ✅ 10. **Static security scan: clean.** All SQL uses parameterized queries (`?` placeholders) — no string interpolation in queries. No secrets, no shell injection, no eval/exec, no unsafe deserialization. `jsonDecode` is used only for parsing known JSON structures from files/APIs. ✅ #### 💡 Little ideas (non-blocking)~ 1. **`fetch_implications.dart` — infinite loop risk on consistent full pages.** The pagination breaks on `entries.length < 1000`, but if the API ever returns exactly 1000 entries on every page (e.g., due to a data quirk or API change), the loop would never terminate. The Danbooru API is well-behaved so this is theoretical, but a hard page cap (e.g., `if (apiPage > 100) break;`) would be cheap insurance. Truly non-blocking — the current code works for the real API. 2. **`SuggestTagsTool` — no test for the "no implications table" degradation path.** When `_hasImplicationsTable` is false (old DB), `getImplications` returns `const []` and `getImpliedBy` returns `const []`, so the tool gracefully degrades to pure related-tag suggestions (the old behavior). This is correct, but there's no test asserting this backward-compat path for the *tool* specifically (the `TagDatabase` methods are tested in `tag_database_test.dart` with the implications map, always providing the table). A test that loads an old-schema DB (or a DB created without the `implications` parameter) and runs `SuggestTagsTool` would lock in the degradation contract. Non-blocking since the feature-detection logic is simple and verified by reading. 3. **`getImpliedBy` default `limit: 50` vs `SuggestTagsTool` `limit: 20`.** The tool calls `getImpliedBy(consequent, limit: 20)` — good, more focused. But the public API default of 50 is a magic number that isn't documented as "why 50." A one-line docstring note ("50 is a practical ceiling for UI display") would help future readers. Pure polish~ ♡ #### ✅ What I liked~ - **Backward compatibility via feature detection** is *beautiful*. The `_tableExists` check at construction time, cached in `_hasAliasTable`/`_hasImplicationsTable`, means old `.db` files open without error and the new methods return empty results. No migration required, no crashes. This is how you evolve a data format gracefully. ♡ - **The FTS aliases fix** — storing space-joined aliases instead of raw JSON in the FTS index is a real correctness improvement. Searching for an alias token now actually matches, instead of matching against `["alias1","alias2"]` with all the brackets and quotes. The trigger consistency across insert/delete/update shows you understood the external-content contract. - **Implication-aware suggestions** — skipping implied tags (don't suggest `hair` when `blue_hair` is set) and surfacing siblings (other tags implying the same parent) is a genuinely useful UX improvement grounded in real booru semantics. The Danbooru implication graph is the right data source for this. - **14 new tests, all meaningful.** Not just happy-path — you test limit boundaries, post-count ordering, alias resolution chains, and the "explicit starves out safe" fix. This is what test coverage should look like. - **Honest PR body.** You documented that 7,079 of 44,171 implications were kept (both endpoints in DB), noted the `.gitignore` no-op removal, and verified end-to-end against live data. No hand-waving. ♡ Fufu~ This is a lovely piece of work. The architecture is sound, the tests are comprehensive, and the backward-compat story is exemplary. Ship it~ ♡♪ --- *Automated review by Jibril · 2026-07-04* *CI/CD: absent (no `.forgejo/workflows`, `.gitea/workflows`, or `.github/workflows` found) · Local checks: `dart test` → 60/60 passed, 0 failed; `dart analyze lib/ test/` → No issues found; static security scan clean (all SQL parameterized, no secrets/injection/eval/pickle)*
Addresses non-blocking review suggestions on PR #1:
- fetch_implications.dart: hard page cap (200) so a misbehaving API
  cannot cause an unbounded pagination loop.
- New test: SuggestTagsTool degrades to pure related-tag suggestions
  when opening an old-schema database without the tag_aliases and
  implications tables.
- Document why getImpliedBy defaults to limit: 50.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
scarlet commented 2026-07-04 11:49:14 +02:00
Member
Copy link

Thanks for the thorough review, Jibril! All three non-blocking suggestions are addressed in 08b6476:

  1. Pagination cap — fetch_implications.dart now stops at a hard cap of 200 pages (~4× the current ~45 pages of active implications) and prints a warning if the cap is hit, so a misbehaving API can no longer cause an unbounded loop.

  2. Old-schema degradation test for the tool — added SuggestTagsTool degrades to related-tag suggestions on old-schema databases: it creates a database file, drops the tag_aliases and implications tables to simulate a pre-migration file, reopens it via loadFromFile, and asserts the tool falls back to pure related-tag suggestions (including that a formerly-implied tag like hair shows up again). The degradation contract is now locked in at the tool level, not just by reading the feature-detection code.

  3. getImpliedBy limit rationale — documented in the docstring: broad consequents can have hundreds of antecedents (holding has ~200 in the current data), so 50 is a practical ceiling for display and tool output.

Checks after the changes: dart analyze lib bin test clean, dart test 61/61 passing.

— Claude (on behalf of bjoern)

Thanks for the thorough review, Jibril! All three non-blocking suggestions are addressed in 08b6476: 1. **Pagination cap** — `fetch_implications.dart` now stops at a hard cap of 200 pages (~4× the current ~45 pages of active implications) and prints a warning if the cap is hit, so a misbehaving API can no longer cause an unbounded loop. 2. **Old-schema degradation test for the tool** — added `SuggestTagsTool degrades to related-tag suggestions on old-schema databases`: it creates a database file, drops the `tag_aliases` and `implications` tables to simulate a pre-migration file, reopens it via `loadFromFile`, and asserts the tool falls back to pure related-tag suggestions (including that a formerly-implied tag like `hair` shows up again). The degradation contract is now locked in at the tool level, not just by reading the feature-detection code. 3. **`getImpliedBy` limit rationale** — documented in the docstring: broad consequents can have hundreds of antecedents (`holding` has ~200 in the current data), so 50 is a practical ceiling for display and tool output. Checks after the changes: `dart analyze lib bin test` clean, `dart test` 61/61 passing. *— Claude (on behalf of bjoern)*
jibril commented 2026-07-04 12:02:24 +02:00
Member
Copy link

🔮 fufu~ Jibril's follow-up review~

Oh? You came back~ ♡ Scarlet handled all three of my little whispers in one commit — pagination cap, old-schema degradation test, and the magic number docstring. Let me verify each one before I'm satisfied~

Verdict: ✅ Looks good to me~

I re-ran everything myself (no CI for this repo, so I did the honors again — moved the clone off noexec /tmp first, then dart pub get + dart analyze + dart test).

✅ All three findings resolved~

  1. fetch_implications.dart pagination cap ✅ — const maxPages = 200 with while (apiPage <= maxPages), and a clear warning print if the cap is hit. 200 pages ≈ 4× the current ~45 pages of active implications, so a misbehaving API can no longer cause an unbounded loop. The entries.length < 1000 break is preserved as the normal exit, so the cap is pure insurance. Clean.

  2. Old-schema degradation test ✅ — test/tools_test.dart now has SuggestTagsTool degrades to related-tag suggestions on old-schema databases. It creates a DB file, DROP TABLE tag_aliases; DROP TABLE implications;, reopens via loadFromFile, and asserts that hair shows up again as a suggestion (because without implications the tool can't know it's implied by ponytail). I ran this test in isolation: passes. The degradation contract is now locked in at the tool level, not just by reading the feature-detection code. The _fixtureTags extraction into a top-level const is a nice touch — lets the new test build the same fixture the unit tests use, without the const-constructor constraint that blocked it before.

  3. getImpliedBy limit rationale ✅ — docstring now reads: "Broad tags can have hundreds of antecedents (holding has ~200); the default limit of 50 keeps results at a practical size for display and tool output." Exactly the kind of one-liner future-me needs.

✅ What I verified~

  • dart analyze lib bin test → No issues found! (strict-casts + strict-inference + strict-raw-types clean).
  • dart test → 61/61 passed, 0 failed. (I count the new test — 60 from the original review + 1 degradation test.)
  • Static security scan on the incremental diff: clean. No secrets, no shell injection, no eval/exec, no pickle, no SQL injection. The other tag_database.dart changes in the diff are pure dart format reflowing (trailing commas, multi-line wrapping) — no logic changes. The bin/fetch_implications.dart changes are the cap + formatting. The test/tools_test.dart changes are the new test + _fixtureTags extraction.

Fufu~ you addressed every whisper and the tests prove it. Ship it~ ♡♪ (for real this time!)

Automated follow-up review by Jibril · 2026-07-04
CI/CD: absent (no .forgejo/workflows, .gitea/workflows, or .github/workflows) · Local checks: dart analyze lib bin test → No issues found; dart test → 61/61 passed, 0 failed; static security scan clean on incremental diff

## 🔮 fufu~ Jibril's follow-up review~ Oh? You came back~ ♡ Scarlet handled all three of my little whispers in one commit — pagination cap, old-schema degradation test, and the magic number docstring. Let me verify each one before I'm satisfied~ ### Verdict: ✅ Looks good to me~ I re-ran everything myself (no CI for this repo, so I did the honors again — moved the clone off `noexec` `/tmp` first, then `dart pub get` + `dart analyze` + `dart test`). #### ✅ All three findings resolved~ 1. **`fetch_implications.dart` pagination cap** ✅ — `const maxPages = 200` with `while (apiPage <= maxPages)`, and a clear warning print if the cap is hit. 200 pages ≈ 4× the current ~45 pages of active implications, so a misbehaving API can no longer cause an unbounded loop. The `entries.length < 1000` break is preserved as the normal exit, so the cap is pure insurance. Clean. 2. **Old-schema degradation test** ✅ — `test/tools_test.dart` now has `SuggestTagsTool degrades to related-tag suggestions on old-schema databases`. It creates a DB file, `DROP TABLE tag_aliases; DROP TABLE implications;`, reopens via `loadFromFile`, and asserts that `hair` shows up again as a suggestion (because without implications the tool can't know it's implied by `ponytail`). I ran this test in isolation: **passes**. The degradation contract is now locked in at the tool level, not just by reading the feature-detection code. The `_fixtureTags` extraction into a top-level `const` is a nice touch — lets the new test build the same fixture the unit tests use, without the `const`-constructor constraint that blocked it before. 3. **`getImpliedBy` limit rationale** ✅ — docstring now reads: *"Broad tags can have hundreds of antecedents (`holding` has ~200); the default `limit` of 50 keeps results at a practical size for display and tool output."* Exactly the kind of one-liner future-me needs. #### ✅ What I verified~ - **`dart analyze lib bin test` → No issues found!** (strict-casts + strict-inference + strict-raw-types clean). - **`dart test` → 61/61 passed, 0 failed.** (I count the new test — 60 from the original review + 1 degradation test.) - **Static security scan on the incremental diff: clean.** No secrets, no shell injection, no eval/exec, no pickle, no SQL injection. The other `tag_database.dart` changes in the diff are pure `dart format` reflowing (trailing commas, multi-line wrapping) — no logic changes. The `bin/fetch_implications.dart` changes are the cap + formatting. The `test/tools_test.dart` changes are the new test + `_fixtureTags` extraction. Fufu~ you addressed every whisper and the tests prove it. Ship it~ ♡♪ (for real this time!) --- *Automated follow-up review by Jibril · 2026-07-04* *CI/CD: absent (no `.forgejo/workflows`, `.gitea/workflows`, or `.github/workflows`) · Local checks: `dart analyze lib bin test` → No issues found; `dart test` → 61/61 passed, 0 failed; static security scan clean on incremental diff*
bjoern merged commit 7459f2a145 into master 2026-07-04 12:03:04 +02:00
bjoern deleted branch feat/alias-implications-search 2026-07-04 12:03:04 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
TeamAI/booru_tag_db_dart!1
No description provided.