finish

name	finish
description	End-of-task pipeline — simplify review, double review, tests, single analyze+test gate, changelog/docs. Use when you've finished implementing a task and want to harden it before committing. Does NOT commit or push; the user must ask explicitly.

Finish Pipeline

Run this once at the end of a task. The pipeline merges the old simplify / double-review / full-coverage-tests / changelog-docs skills into a single flow with a single analyze+test gate at the end.

Ground rules

• Collect the diff once at Phase 0; reuse it across phases.
• Do NOT run flutter analyze or flutter test between phases — only Phase 5 runs them.
• Do NOT commit or push unless the user explicitly asks.
• Do NOT create new .md files unless the user explicitly asks.
• Fix findings inline within the phase that surfaced them; don't park lists of "TODO later".

Comment style (enforced project-wide)

• English only. No Russian / Cyrillic in ///, //, or /* */ blocks. UI strings stay in .arb; this rule is about code comments.
• Only WHY, never WHAT. A comment exists to explain a hidden constraint, a subtle invariant, a workaround, a non-obvious algorithm choice, or a refactor note. Names and types already cover what.
• No comment > bad comment. If you can't say something the code can't, delete the comment.
• Hard cap: 2 lines. No comment block (//, ///, or /* */) may exceed 2 consecutive lines — anywhere, including dartdoc on public API. If it doesn't fit in 2 lines, cut it until it does.
• No file-header comments. A file never starts with a comment block describing what the file is — the path and the first declaration already say that. Delete top-of-file comments in every new or touched file.
• No section dividers, no banners. // ===== Foo ===== and // --- bar --- are noise — drop them.
• No TODOs without a tracker reference. A bare // TODO: fix this rots; tie it to an issue or remove it.
• Translate, don't preserve. When touching a file with Russian comments, translate the salvageable ones to English and delete the rest as part of the same diff.

Phases

Phase 0 — Snapshot

git status --short
git diff HEAD

Enumerate changed files once. If the diff is empty, stop with "nothing to finalise".

Phase 1 — Simplify

Review new/changed code from three independent angles. This phase is self-contained — do not delegate to a built-in /simplify skill, which may not exist in every Claude Code version. Run the review yourself using these three lenses:

1. Reuse

   • Search the codebase for existing utilities, DAO methods, helpers, or constants that could replace newly written code. Common locations: lib/shared/, sibling files of the changed file, existing DAOs.
   • Flag new functions that duplicate existing functionality.
   • Flag inline logic that could use an existing helper (string manipulation, path handling, type guards, platform checks).

2. Quality

   • Redundant state, cached values that could be derived.
   • Parameter sprawl (adding N-th param instead of restructuring).
   • Copy-paste with slight variation → unify with shared abstraction.
   • Leaky abstractions, stringly-typed code where enums/consts exist.
   • Unnecessary wrapper widgets/Boxes that add no layout value.
   • Nested conditionals 3+ levels (flatten with early returns, lookup tables, if/else-if cascade).
   • Unnecessary comments explaining WHAT (delete; keep only WHY for hidden constraints, subtle invariants, workarounds).

3. Efficiency

   • Redundant computations, duplicate network/DB calls, N+1 patterns.
   • Missed concurrency: independent awaits in sequence → Future.wait.
   • Hot-path bloat (startup, per-render, per-request).
   • No-op store updates in polling loops / event handlers (add change-detection guard).
   • TOCTOU anti-pattern (pre-checking existence then operating) → operate directly and handle the error.
   • Unbounded data structures, missing cleanup, event listener leaks.
   • Reading whole files when a slice is enough.

For large diffs (≥5 changed files or ≥300 lines), dispatch the three angles as parallel Agent sub-tasks (subagent_type: Explore) in a single message — each agent gets the full diff and reports its findings in ≤300 words. Then aggregate and fix. On small diffs, review all three angles yourself inline.

Fix actionable issues. Skip false positives — don't argue, just move on. Out-of-scope issues (pre-existing cruft in untouched files) get noted in the final report, not fixed here.

Phase 2 — Double review

R1 — correctness

• Logic, edge cases, strict typing (no dynamic, no var in public API, final / const where possible, nullable handled via ?. / ?? / checks).
• Error handling (no silent failures).
• SharedPreferences per-profile rule: user-specific keys MUST be suffixed with '${profileId}' read via a getter (not cached in a field — profile can change). Global keys (API creds, theme, language) are the exception.
• Cross-platform: no hardcoded Windows paths, package:path for joins, Platform.isX branches fall through for all supported OS (Windows + Linux + Android), Windows-only plugins (webview_windows) gated by kVgMapsEnabled.
• New enum value propagation (only if the change adds a MediaType / CanvasItemType value): run flutter analyze to surface exhaustive-switch errors across canvas_view.dart, all_items_screen.dart, collection_screen.dart, export_service.dart, import_service.dart, plus filter chips, CollectionStats counts, collectedXxxIdsProvider, and all localisation keys (unknown*, allItems*, collectionFilter*, mediaType*, searchSource*).
• DB migrations (any schema change): create*Table in schema.dart is shared between fresh installs (createAll) and the historical migration that first created the table — so any column or index baked into a create*Table is also re-applied when an old DB jumps many versions in one upgrade (the create runs with today's schema, then the historical ALTER / CREATE INDEX runs on top). A raw statement then throws duplicate column name / index already exists and the whole upgrade rolls back, leaving the app frozen on the splash. Rules:
  • Adding a column: update the relevant create*Table in schema.dart (fresh installs need it) add a new migration that calls — a bare .

R2 — quality and performance

• Readability (function size, single responsibility, self-documenting names).
• Duplication (shared logic extracted).
• Memory / leaks (subscriptions cancelled, controllers disposed).
• Flutter specifics (const widgets, ListView.builder for long lists, no object creation in build()).

R2b — enum single source of truth (mandatory)

No raw string / number literal in production code may duplicate a value, .name, .key, or display label that an enum already owns. If a concept is modelled by an enum (MediaType, ItemStatus, DataSource, CanvasItemType, CollectionSortMode, ExportFormat, AnimeMangaTitleLanguage, NavTab, DiscoverSectionId, …), every read / write / compare / label / default MUST go through it — EnumX.value, EnumX.name, EnumX.key, EnumX.label, localizedLabel(l), fromString / tryFromString — never a bare 'game', 'completed', 'mangabaka', 'IGDB', 'romaji', 'light'. This is doubly non-negotiable for values that already exist in an enum: the literal must live in exactly one place, the enum.

How to check: for each enum the diff touches (and its close neighbours), grep its literal values and labels across lib/. The only non-exception home should be the enum definition. Also grep the diff itself for quoted strings that echo an enum member — new hardcodes are the common regression.

Fix: replace the literal with the enum accessor (import the enum where needed). If it is a default parameter value (needs const), expose a static const on the enum and reference that (e.g. AnimeMangaTitleLanguage.defaultId). If several call sites hardcode a per-source string (groupId, apiName, sourceName), derive it once from the owning enum (DataSource.key / DataSource.label) and delete the overrides / parallel maps.

Allowed exceptions (do NOT flag these):

• The enum's own definition file — the single place the literal lives.
• DB migrations (lib/core/database/migrations/) and create*Table DDL in schema.dart — historical / storage strings, immutable.
• Raw SQL string bodies — leave the literal; reference the enum in a comment if helpful.
• External-API mapping — the provider's own tokens on the far side of a translation boundary (TMDB movie/tv, AniList ANIME/MANGA, Kitsu / MAL / Trakt / Kinorium, search-filter API values, AniList JSON keys like romaji). These are not the enum's namespace.
• Generated l10n (app_localizations*.dart) and .arb — localisation resources.
• A short badge label that legitimately differs from a full display name — e.g. DataSource.steamGridDb.label is 'SGDB' but the credits screen shows 'SteamGridDB'; these are different concepts, do not force one into the other. Same for a per-tab SearchSource.id ('movies', 'manga') which is a media/tab identifier, not the provider name.

R2c — model purity (mandatory)

lib/shared/models/** stays pure Dart: no package:flutter, no dart:ui, no l10n imports — direct or transitive through other model files. Models hold data only (ARGB colors as int, hex colors as String, stored enum values); presentation (Color, IconData, localized labels) lives in extension <Model>Ui files under lib/shared/constants/*_ui.dart, hex⇄Color codecs in lib/shared/utils/color_hex.dart. Rationale: the model layer is slated for extraction into a pure-Dart core package shared with the selfhost server, and dart:ui does not exist in a plain Dart VM.

How to check (must return nothing):

grep -rln "package:flutter\|dart:ui\|l10n/" lib/shared/models/

Fix: move the offending getter/method into the model's *_ui.dart extension (create it if missing), store the raw value in the model, and add the extension import at call sites. Never "fix" by re-adding a Flutter type to a model.

R2c-rpc — regenerate the RPC layer (mandatory when the diff touches a DAO or a model)

packages/core/tool/generate_rpc.dart emits the browser-side stubs, the per-DAO dispatchers and the dispatch table from the DAO signatures and every model those signatures reach. So a field added to Collection, Game, CollectionItem — anything a DAO returns — changes the wire format while changing no signature at all: nothing in the type system sees it, and the stale stub silently drops the field.

If the diff touches packages/core/lib/database/dao/** or packages/core/lib/models/**, regenerate and commit the result:

powershell.exe -Command "cd '$(wslpath -w "$PWD")/packages/core'; dart run tool/generate_rpc.dart"

Do not hand-edit anything under packages/core/lib/rpc/generated/. The Phase 5 gate proves it: packages/core/test/rpc/generated_up_to_date_test.dart regenerates in memory and diffs against the committed files, so a forgotten run fails dart test.

New wire shape the generator refuses (No wire rule for X)? Teach it a rule in _encode / _decode and cover it with a round-trip test in packages/core/test/rpc/ — never work around it by degrading the DAO signature.

R2d — web readiness (mandatory)

The project is headed for a selfhost web build: same branch, web as one more build target, DAO calls become the client↔server RPC boundary, external APIs go through a server proxy. New code must not create rework for that plan. Check the diff for:

• No new unguarded dart:io (Platform.is*, File, Directory, Process) in lib/features/ or lib/shared/. Platform branching goes through platform_features.dart flags; intrinsic file I/O (export/import, disk cache) stays behind existing service boundaries or a flag so web can stub it. dart:io inside lib/core/services/ that a web build will conditionally replace is acceptable; a Platform.isWindows inline in a widget is not.
• DB access only through DAO methods. No raw SQL or db.rawQuery outside lib/core/database/dao/ — every DAO method is a future RPC endpoint, so the UI/provider layer must call dao.method(...), never touch the Database handle. New DAO method signatures must be JSON-serialisable at the boundary: arguments and returns built from primitives, enums (sent as .name/.value), DateTime, models with toDb/fromDb, and collections thereof — no callbacks other than the established _getDatabase injection, no Database/Transaction parameters in public signatures, no returning raw Map rows where a typed record/model is feasible.
• External API calls stay inside lib/core/api/ clients (Dio) — never a one-off http/Dio call from a widget or provider; the proxy phase swaps base URLs in one place.
• dart:ui / package:flutter stay out of models and pure-logic layers (overlaps R2c) — the server imports these files in a plain Dart VM.
• Web-incompatible plugins (webview_windows, window management, gamepad, Discord RPC, file pickers) — any new usage must sit behind a platform_features.dart flag, not a bare platform check.

Fix: route the platform check through a flag, move raw SQL into a DAO method, move the HTTP call into the API client. Flag (don't silently accept) anything that would force the selfhost phases to redesign the new code.

R2e — theme awareness (mandatory)

The app has switchable themes (AppPalette.dark / AppPalette.sakura, selected in Settings → Appearance). Every color decision must survive both a near-black and a near-white background. Check the diff for:

• No hardcoded colors in UI code. No Colors.* (except Colors.transparent) and no inline Color(0x...) in widgets — every color goes through an AppColors token, which reads the active palette. Semantic tokens for the common traps: over poster/image art use AppColors.scrim (+withAlpha at the call site) and AppColors.onOverlay; text/icons on brand-filled controls use AppColors.onBrand; drop shadows AppColors.shadow; modal barriers AppColors.barrier or scrim.withAlpha(...). Never assume "white text reads fine" — the background may be #FDF2F4.
• Allowed exceptions (do NOT flag): external-brand colors (platform_ui.dart, service_badges.dart, RA/Discord brand constants, the settings capsule _k*Colors), color-picker swatch palettes (user content), luminance-based black/white contrast picks (luminance > 0.5 ? black : white), pure alpha masks ([white, transparent] shader gradients).
• No cached theme colors. Never store anything derived from AppColors.* / AppTypography.* / MediaTypeTheme.* in a static final, static const, top-level final, or a const constructor default — the value freezes on the palette active at first access and survives a theme switch. Use a getter (static Color get x => AppColors.y;) or compute inside build(). Grep the diff for static final/top-level final whose initializer mentions those classes.
• New color = new palette entry. A genuinely new color goes into AppPalette as a field with a value for every palette (dark AND sakura — pick a sakura shade with real contrast on the light background, usually a darkened variant), plus a delegating AppColors getter. Never add a color that exists in only one theme.
• New screens/widgets in the diff: reason through (or run) both themes — what does every used token render to in sakura? Pale-on-light and white-on-white are the recurring regressions.

R3 — localisation

• Every UI string uses S.of(context).key or final S l = S.of(context);.
• ARB: every key exists in every lib/l10n/app_*.arb locale file (glob them — the set grows over time: en, ru, zh, …); placeholder names match across all of them; Russian plurals use ICU =0 / =1 / few / other. Languages without plural forms (e.g. Chinese) may render an ICU-plural key as a single flat string ({count} 项) as long as they keep the same placeholders.
• Enum labels via localizedLabel(S l) extensions, not raw .displayLabel.
• Status labels adapt to media type (Playing for games, Watching for movies/TV).
• Allowed English: debug screens, debugPrint, model field names, enum .name, test assertions.

Phase 3 — Tests for new code

Goal: useful tests, not coverage theatre. 100% line/branch coverage is not the target. Aim for tests that break only when real behaviour breaks — and that would catch a future regression you'd actually care about.

Every test must pull its weight in one of three buckets:

1. UI doesn't silently break

• Widget renders without exceptions (expect(tester.takeException(), isNull)).
• Critical flows work: tap handlers fire the right callback, navigation happens, conditional widgets show/hide on state change, lists render the right item count.
• Do NOT test colours, text labels, icons, font sizes, padding, border radius, widget types used purely for styling (e.g. find.byType(Container)). Design changes must not break tests. Localised string values — same rule: assert that some text appears, not that it equals a specific string.
• Data-driven text that flows from model → UI is fair game (expect(find.text(collection.name), findsOneWidget) is OK; expect(find.text('Collections'), findsOneWidget) for a static title is not).

2. Logic is verified reliably

• For every public method / function: happy path + each meaningful branch (if/else, switch cases, early returns, error paths).
• Edge cases the change actually cares about: empty input, null, boundaries, concurrent-state races where relevant. Skip defensive null-guards that can't trigger in practice.
• Model serialisation round-trips (fromJson/toJson, fromDb/toDb) when the change touches models.
• copyWith semantics when a new field is added.

3. Method calls at the boundary are verified

• When the change orchestrates multiple collaborators (DAO, repository, API client, provider), use verify(() => mock.method(args)).called(N) to pin down that the right method was called with the right args, the right number of times.
• Use verifyNever to assert negative-space guarantees (e.g. "no tag remap when sourceTagId is null").
• Use captureAny() to inspect complex payloads (e.g. "the cloned row has tag_id: null").

Infrastructure rules (non-negotiable):

• import '../../helpers/test_helpers.dart' — reuse mocks from test/helpers/mocks.dart, builders from builders.dart, fallbacks via registerAllFallbacks() in setUpAll.
• Mock/builder will be used in ≥2 files → add it to helpers. Unique to one test → declare locally.
• Widget tests use tester.pumpApp(), not a hand-rolled ProviderScope + MaterialApp.
• Naming: should [expected result] when [condition].

Self-check before finishing a test: "If someone changed the design tomorrow (colours, labels, layout) — would this test fail?" If yes, and the change wasn't a logic change, the test is overfitted. Remove or relax it.

Phase 4 — Changelog + docs

CHANGELOG.md — [Unreleased] section, Keep a Changelog version headers (## [Unreleased] / ## [X.Y.Z] - date) with Added / Changed / Fixed / Removed sub-sections; inside each sub-section, entries follow GNU Change Log style.

Entry structure — three parts separated by blank lines:

1. Topic line — one bolded sentence summarising the change (past-tense or imperative, like a commit subject).
2. Body (optional, 1–3 sentences) — what the user sees and why. Plain prose, no file paths. Skip if the topic line is self-explanatory.
3. File list — bulleted index of affected files in the form * path/to/file.dart (ClassName.methodName, OtherSymbol): what changed. Use full paths from the repo root and full symbol names (never abbreviate, never group with {foo,bar} syntax — every symbol must be greppable on its own). Several files with identical descriptions can be combined on one line separated by commas. A file with no specific symbol worth naming can be listed as * path/to/file.dart: what changed.

Rules:

• English entries. No Russian abbreviations or Cyrillic shorthand in English prose (write "right-click", not "ПКМ"; "left-click", not "ЛКМ"). Russian strings quoted as UI labels ("«Желаемое»") are fine — that's data, not prose.
• Separate unrelated topics with a blank line (already enforced by Markdown list spacing).
• Unreleased consolidation: when enhancing or fixing something already in [Unreleased] that was never released, update the existing entry in place — don't add a separate Fixed / Changed bullet. Users should see the final state, not the development history. This applies to the topic line, body, and the file list alike.
• If a single topic spans many unrelated files (≈30+), it probably bundles several changes — split into multiple topic entries rather than letting one file list balloon.

Example:

- **Expand AniList search filters for anime and manga**

  Anime tab grows from 2 filters to 4; manga from 2 to 4. Multi-select
  genre uses OR match. Year filter uses `startDate` bounds so it works
  for older and cancelled titles where `seasonYear` is null.

  * lib/core/api/anilist_api.dart (AniListApi.browseAnime, AniListApi.browseManga):
    Change `$genre: String` → `$genres: [String]`; add `$format`, `$status`,
    `$startDateGreater`, `$startDateLesser` GraphQL vars.
  * lib/features/search/filters/anilist_anime_format_filter.dart
    (AniListAnimeFormatFilter), anilist_manga_status_filter.dart
    (AniListMangaStatusFilter): New.
  * lib/features/search/filters/manga_format_filter.dart (MangaFormatFilter.options):
    Limit to MANGA, NOVEL, ONE_SHOT — MANHWA / MANHUA / LIGHT_NOVEL were
    rejected by AniList's `MediaFormat` enum.

docs/ — update only if the change actually affects them:

File	When to touch
ARCHITECTURE.md	New layer, major module, or shift in patterns. Keep it a high-level map — do NOT add per-file tables or SQL schema dumps
CONTRIBUTING.md	Changes to development process
CODESTYLE.md	New lint rules, typing conventions
COMMITS.md	Changes to commit conventions
RCOLL_FORMAT.md	Changes to .xcoll / .xcollx export format
GAMEPAD.md	New focusable widgets, navigation rules
SNACKBAR.md	Changes to context.showSnack() API or types

Language per file: most are Russian. Keep each in its current language. Preserve formatting — make targeted edits, don't rewrite.

Phase 5 — Gate (single run)

powershell.exe -Command "cd '$(wslpath -w "$PWD")'; flutter analyze --fatal-infos --fatal-warnings"
powershell.exe -Command "cd '$(wslpath -w "$PWD")'; flutter test"
powershell.exe -Command "cd '$(wslpath -w "$PWD")/packages/core'; dart test"
powershell.exe -Command "cd '$(wslpath -w "$PWD")/server'; dart test"

packages/core and server resolve separately, so a change there is invisible to flutter test — run all four.

Follow the failure-recovery rules below. When green, STOP — report what changed and wait for an explicit commit/push instruction.

Failure recovery

Failure	Response
Analyzer fails	Fix inline. Re-run analyzer only. Do not re-do earlier phases.
Test fails — a test I just wrote	Fix the test (wrong mock stub, missing fallback, wrong assertion). Re-run tests only.
Test fails — existing test	Default: the test is right, the production code is wrong. Do not edit the test yet. First, re-read the test and the code paths it covers. Ask: "Was this specific behaviour something I deliberately changed as part of the task?" Answer this honestly before touching anything. → If NO (surprise failure, behaviour change you didn't plan): back to Phase 1 — the code is wrong, fix the code, then Phase 3 for the affected area, then re-gate. → If YES (the old assertion contradicts the intended new behaviour, and the new behaviour is in the spec/user request): update the test, rerun tests. Document the behaviour change in the final report so the user sees what shifted. If unsure, default to NO.
Review (Phase 1-2) needs a code change	Fix inline. If the fix touches production code (not just comments/docstrings), add/update tests in Phase 3 before re-gating.
R3 reveals missing ARB keys	Add the key to every lib/l10n/app_*.arb locale file (glob them, don't assume a fixed set), run powershell.exe -Command "cd '$(wslpath -w "$PWD")'; flutter gen-l10n", re-run analyzer.
generated_up_to_date_test fails	The RPC layer is stale — regenerate (dart run tool/generate_rpc.dart in packages/core) and commit the output. Never edit the generated files to make it pass.
Flaky test	Retry the affected test file once via flutter test path/to/test.dart. If it still fails, treat it as real.

Anti-loop rule — if the same error has been attempted twice with different fixes and still fails, STOP and report to the user. Do not keep hacking.

Scope creep — unrelated issues discovered during review (pre-existing bugs, cruft in untouched files) are NOT fixed here. Note them in the final report; user decides.

Report format

One line per phase as it completes, e.g.:

Phase 1 simplify: 2 fixes (removed redundant setItemTag write, inlined unused helper).
Phase 2 R1 critical fix: SQLite LOWER() doesn't handle Cyrillic → switched to Dart toLowerCase. R2/R3 clean.
Phase 3 tests: 20 new tests across 3 files, all green.
Phase 4 changelog: 1 Changed entry; no docs/ touched.
Phase 5 gate: analyze clean, 4776 tests passed.
Risk: Low — re-downloadable cache only, no permanent data loss; affects the Settings cache button.
Status: ready for commit. Awaiting explicit /commit.

Phase 6 — Risk assessment & impact (always, after the gate)

A green gate proves the code compiles and the tests pass — it does not prove the change is safe to put in front of a real user. Close every run with a short, honest risk assessment so the user can decide whether to ship. Never rubber-stamp; if you can't find a risk, say why the change is inherently safe (pure addition, read-only, behind a flag), don't just assert "looks fine".

Cover four points, one or two lines each:

1. Blast radius — what it affects. The features, flows, screens, files, and stored data this change touches, and who is hit: all users, one platform (Windows / Android), or only users with a precondition (feature X enabled, a custom data folder, a specific profile, a populated cache/DB). Name the concrete surface, not "the app".

2. End-user safety. Can it lose or corrupt user data, delete files, break a DB migration, or wedge the app on the splash screen? Is the effect reversible (undo, re-download, re-sync from source) or permanent? Anything irreversible is called out explicitly. If the change deletes or overwrites anything on disk or in the DB, this point is mandatory, and you must distinguish destructive-but-recoverable (e.g. re-downloadable cache) from destructive-and-permanent (e.g. the only copy of a user upload).

3. Worst-case failure. If the change is subtly wrong despite the tests, what does the user actually experience — cosmetic glitch → broken flow → crash → silent data loss — and how likely is that given what the tests in Phase 3 actually pin down. Be specific about which failure modes the tests do not cover.

4. Bottom line. One rating — Low / Medium / High risk — with a one-sentence justification, plus any "watch this in the wild" notes: edge cases left untested, platform-specific behaviour, interactions with the data folder / profile / cache / migration chain.

Keep it factual and scoped to this diff. The bottom-line rating goes on the Risk: line of the report (see above); the four points expand it underneath when the change is anything more than trivial.