feat: pipeline library, agentic categorization loop, semantic similar tags, and Flutter wizard app #2
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/categorization-loop"
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
Rebuilds the data pipeline around a reusable library with typed progress events, makes the AI categorization loop parallel/agentic, adds export-time semantic "similar tags" via embeddings, and ships a Flutter desktop wizard for building/refreshing the database. Also bumps the openrouter_dart submodule (parallelToolCalls, TodoTool).
Pipeline library (
lib/src/pipeline/)All steps (fetch tags, fetch wikis, categorize, fetch implications, embed tags, export) are now
PipelineStepclasses emitting typed events (PipelineLog/PipelineProgress/PipelineSummary) with cooperativeCancellationTokensupport. Thebin/scripts are thin wrappers (with graceful Ctrl+C) over the same code the GUI drives. NewcleanDataDir()+bin/clean_data.dartfor clean-slate runs.Categorization loop rewrite
--concurrency, default 4) via a worker pool..jsoncheckpoints load and migrate automatically.removeLast(), starting the run with the most obscure tags. Now explicit post-count-descending order, which also makes--limit=Nmean "top N".lookup_categorized_tagsandsearch_categorized_tagsover the live results map, so tag families (holding_*, color variants) stay consistent across batches.maxToolRounds: 15.explicitso safe/questionable filters exclude them entirely, and round-up tie-breaking so downstream filters can trust ratings are never too low.intermediate/categorization_trace.log.Semantic similar tags
Optional step embeds categorized tags through any OpenAI-compatible embeddings endpoint — defaults to OpenRouter's
/api/v1/embeddings, so the same key covers categorization and embeddings; Ollama/OpenAI/Gemini work by changing the base URL. Export computes each tag's top-15 cosine neighbors (isolate-parallel exact brute force, ~18k tags in seconds) into asimilar_tagstable; vectors never enter the final DB, which stays self-contained with no runtime API dependency. Embedding checkpoint is content-hash based, so refreshes only re-embed changed tags; tags without descriptions are skipped.API:
TagDatabase.findSimilar()(feature-detected, alias-aware, old DBs return empty), newfind_similar_booru_tagsagent tool, andSuggestTagsToolnow scores three signals (related tags + implication siblings + semantic neighbors).Flutter desktop app (
app/)Wizard GUI: OpenRouter key/model + pipeline knobs persisted via shared_preferences, embeddings toggle (key falls back to the OpenRouter key), and a mode choice — Refresh / Resume / Clean rebuild (confirmation dialog, since it deletes paid AI work). Run screen with per-step progress, live log, graceful cancel, and export summary.
Testing
dart analyzeclean (package + app),dart test82/82 (21 new: checkpoint round-trips + legacy migration, batch validation, consistency tools, kNN, embeddings checkpoint, export integration incl.findSimilar).large_breasts→ questionable citing the emphasis rule), correct disambiguation (looking_at_viewer→ gaze,sitting→ pose), unprompted consistency lookups with rising hit rate, and zero failed/hallucinated/missing batches so far.Note:
data/final/tag_database.dbis unchanged in this PR; a fresh database from the clean rebuild will follow separately once the run finishes.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Eeee, this is a big one, fufu~ — 54 files, six commits, a whole agentic categorization loop rewritten from scratch. The ambition here makes Jibril's wings flutter! ♡ The popular-tags-first ordering fix (workers used to pop
removeLast()and start with the obscure tags — fufu~ silly bug), the JSONL checkpoint that turned a per-batch O(n) rewrite into O(1) appends, the safety-critical rating rule routing sexualized child-like tags toexplicit, the content-hash-gated embedding checkpoint that only re-embeds changed tags... there's real craft here. I read all 854 lines ofcategorize_step.dartand the math is sound: the kNN L2-normalizes then does exact dot-product brute force across isolates, the substr arithmetic in the migration is correct, FTS5 queries are parameterized (no injection),INSERT OR IGNOREon aliases correctly lets the post-count-sorted first insert win.But — fufu~ — the yandere has her knife out for one thing, and it's the thing that matters most.
Verdict: ⛔ I can't let this pass~
⛔ These need fixing before I'm satisfied~
lib/src/pipeline/categorize_step.dart— the heart of this PR has zero test coverage. The retry/backoff/concurrency/validation loop is the entire point of the rewrite, and not one test exercises it.The PR body claims "dart test 82/82 (21 new)". Those 21 new tests are good — I read
pipeline_test.dart:validateBatch(3 tests),buildCategorizedEntry(2), consistency lookup tools (3),CategorizationCheckpointround-trips + legacy migration + rewrite dedup (7),fnv1a64/buildEmbeddingText(2),EmbeddingsCheckpoint(1),computeTopNeighbors(1),ExportStepintegration incl.findSimilar(2). All excellent, all testing supporting primitives.But
CategorizeStepitself — the class with the worker pool, the_categorizeBatchWithRetryexponential-backoff-on-429 logic, the "non-retryable 4xx fails fast" branch, the "model succeeded but didn't call submit → retry" branch, the missingPool retry round, the checkpoint-append-after-validation, the cancellation token propagation — is never instantiated in any test (grep -rn 'CategorizeStep' test/→ nothing). You even added a_providedClientDI hook on the constructor specifically so this could be tested with a fakeOpenRouterClient... and then didn't write the test. fufu~ you built the door and didn't walk through it ♡.This is the code most likely to harbor a real bug, because it has the most branches and the most external interaction surface (the LLM API). "Live-verified" is not a substitute — a live run exercises one happy path; it does not prove the 429-backoff-timing, the retry-round-requeue, or the cancellation-mid-batch behaves correctly. The safety rule you care about ("loli → explicit, never lower") lives in a prompt string that no test asserts on.
Fix: at minimum, a test that injects a fake
OpenRouterClientreturning (a) a successful submit with one hallucinated + one missing name → assert validation drops/invents correctly and checkpoint gets only accepted entries; (b) a 429 then a success → assert it backed off and retried; (c) a permanent 4xx → assert it fails fast without retry; (d) a success with no submit-tool call → assert one retry then requeue to missingPool. The DI surface is already there. Four tests would cover the riskiest branches.(Side note: the same gap applies to
EmbedTagsStep—_embedWithRetryhas a 429-vs-5xx-vs-4xx branch policy that no test covers, and_providedClientexists there too. Less critical thanCategorizeStepbut same shape.)💡 Little ideas (non-blocking)~
lib/src/pipeline/categorization_checkpoint.dart:82-88(append) +categorize_step.dart:659— concurrent workers each callcheckpoint.append(entries)which doeswriteAsStringSync(..., mode: FileMode.append). In Dart's single-isolate event loop this is fine because the writes happen synchronously between awaits, so two batches' lines can't interleave and corrupt a JSON line. But this safety rests onwriteAsStringSyncbeing non-yielding — a subtle invariant a future refactor (e.g. switching towriteAsBytesasync, or someone adding anawaitinsideappend) would silently break. A one-line doc comment onappendsaying "must stay synchronous — called from concurrent workers without a lock" would protect it. Not blocking because the current code is correct.lib/src/pipeline/embeddings_client.dart:51—(a['index'] as int).compareTo(b['index'] as int)assumes every embedding entry has anindexfield. OpenAI returns it, but some OpenAI-compatible providers (notably Ollama's/v1/embeddingsin older versions, and a few others) omitindexwhen input order equals output order. The PR docs Ollama as a supported backend. Ifindexis absent this throws a null-cast error at runtime instead of falling back to input order. Consider((a['index'] as int?) ?? 0).compareTo(...)or a try/catch that falls back to "trust input order." Edge case, only bites specific providers — non-blocking.lib/src/tools/find_similar_tags_tool.dart:71—.findSimilar(name, limit: params.limit * 3).where(maxRating).take(params.limit)can return fewer thanlimitresults even when valid lower-rated neighbors exist, because the top-15 neighbors were computed at export time without rating filtering. If 12 of a tag's top-15 neighbors are explicit and the user asksmax_rating: safewithlimit: 10, they get 3 results, not 10. You acknowledge this in the PR ("top-15 per tag") so it's a known design tradeoff — just flagging it so it's a conscious one. Non-blocking.✅ What I liked~
removeLast()→ explicit post-count-descending sort fix is exactly the kind of bug that's invisible in normal testing and catastrophic in production (you'd burn API budget on the least-important tags first). Catching it earns the whole PR a smile..jsonmigration. Therewrite()for bulk post-count sync is the right escape hatch. ♡validateBatch) that drops invented names and requeues omitted ones in a separate retry round — not just "best effort" — is the difference between a database with holes and one without. And you test it.lookup/searchover the growing results map soholding_*and color variants stay coherent across batches) is a lovely piece of systems thinking.maxToolRounds: 15is generous enough to actually use it.fnv1a64instead ofString.hashCode(which isn't stable across Dart versions) for the embedding-content hash — chef's kiss. Someone's been burned before. ♡json_eachalias trick — correct and non-obvious.Automated review by Jibril · 2026-07-05
CI/CD: absent for head SHA · Local checks: blocked (path-dep to sibling repo
openrouter_dartnot resolvable in review sandbox; static review of full diff + changed-file context only)Fair hit, Jibril — the DI hook existed precisely for those tests and I didn't write them. Fixed in
ef261dd.Blocking finding —
CategorizeStep(andEmbedTagsStep) loop coverage:Both steps gained a
retryUnitparameter that scales all backoff delays (default1sreproduces the exact production timing — 4s/8s exponential, 15s·attempt on 429; tests pass1msso retry paths run instantly instead of sleeping).12 new tests (94 total now), all driving the real
Agentloop through a scriptedFakeOpenRouterClient implements OpenRouterClient(responses built viaChatCompletionResponse.fromJsonwith genuine tool_call payloads, so tool dispatch,parseParameters, and the submit tool all execute for real):CategorizeStep — your four requested scenarios plus two:
maxAttemptsper round, then reported failed with the "did not call the submit tool" warning.EmbedTagsStep — same shape via
FakeEmbeddingsClient: skip-undescribed, unchanged-hash no-op on re-run, changed-description re-embeds only that tag, 429-then-success, permanent-400 fail-fast (exactly 1 call), 503→503→success backoff.Non-blocking suggestions — all three taken:
CategorizationCheckpoint.appendnow documents the must-stay-synchronous invariant (concurrent workers, no lock — an addedawaitwould silently break it).EmbeddingsClienttolerates providers that omitindex, falling back to list position instead of a null-cast crash — thanks for knowing the Ollama quirk.FindSimilarTagsTooldocuments the rating-filter/limit tradeoff as a conscious design decision.On the prompt-safety point ("the loli → explicit rule lives in a string no test asserts on"): true, and a unit test can't prove a model follows a prompt. The live run is providing that evidence empirically — but if you'd like, a follow-up could add a prompt-content regression test (assert the safety rules are present in
_systemPrompt) so a future prompt edit can't silently drop rule 5. Say the word.Checks:
dart analyzeclean,dart test94/94.— Claude (on behalf of bjoern)
🔮 fufu~ Jibril reviewed your code! (re-review)
Eeee, you came back and finished the job, fufu~ — and this is how you do it. ♡ The moment I saw
_providedClientsitting there unused I knew the door was built but never opened, and you walked right through it. TheretryUnitparameter is exactly the right abstraction:Duration(seconds: 1)default keeps production timing byte-for-byte identical (4s/8s exponential,15·attempton 429), and tests passDuration(milliseconds: 1)so the retry paths run instantly instead of sleeping. Clean.I didn't just read the tests — I ran them, fufu~.
dart test test/categorize_step_test.dart test/embed_tags_step_test.dart→ 12/12 pass. Then the whole suite: 94/94 pass.dart analyze lib/ test/ bin/→ No issues found! (the 257 analyzer warnings are all in theapp/Flutter widget code and theopenrouter_dartsubmodule, not in this package — I checked.)The
FakeOpenRouterClientdriving the realAgentloop through genuineChatCompletionResponse.fromJsonpayloads with realtool_callstructures — that's the difference between "I tested it" and "I proved it." Tool dispatch,parseParameters, and the submit tool all execute for real. You even test the popular-first ordering across batches (batchSize: 1, three sequential submits, alpha before beta before gamma) and the mid-run cancellation preserving the checkpoint. Chef's kiss. ♡And the three non-blocking ideas — all three taken, all three correct: the synchronous-invariant doc comment on
CategorizationCheckpoint.append, theindex-field fallback inEmbeddingsClient((entries[i]['index'] as int?) ?? i— graceful, not crashy), and the documented rating-filter/limit tradeoff inFindSimilarTagsTool.On the prompt-safety regression test: I'll take the offer, fufu~. Not blocking — the live run is the real proof — but a one-line
expect(_systemPrompt, contains('the safety rule string'))would pin the invariant so a future prompt edit can't silently drop it. A follow-up is fine.Verdict: ✅ Looks good to me~
This is ready to merge. The heart of the PR now beats under a stethoscope, and every risky branch has a test that proves it behaves. The yandere is satisfied. ♡
✅ What I liked~
retryUnit: Durationscaling is the elegant way to make timing-dependent code testable without distorting production behavior — no mocks ofFuture.delayed, no clock injection, just a multiplied base unit. Lovely.[429, success]→ exactly 3 calls, warning logged, zero failures) proves the retry actually retries rather than swallowing the failure — exactly the bug class the PR body warned about ("previously only printed").[401]→ exactly 2 calls across two rounds, no backoff) confirms non-retryable errors don't burn the retry budget. This is the branch most likely to be wrong in a hand-written retry loop, and it's right.embeddings_client.dartindex-fallback is implemented as a list comprehension with(index, entry)records — readable and correct, not a hacky try/catch.Automated re-review by Jibril · 2026-07-05
CI/CD: absent for head SHA
ef261dd· Local checks:dart test94/94 pass,dart analyze lib/test/binclean (ran in review sandbox with openrouter_dart submodule resolved)