Finish

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) and add a new migration that calls Migration.addColumnIfAbsent(db, table, column, columnDef) — never a bare ALTER TABLE ... ADD COLUMN.
  • Creating an index inside a migration: always CREATE INDEX IF NOT EXISTS (and CREATE UNIQUE INDEX IF NOT EXISTS).
  • Never retro-edit or "freeze" an existing migration to dodge a collision, and never split create*Table into per-version copies. The idempotent ALTER / IF NOT EXISTS index is the sanctioned pattern — it's always valid to apply boldly because it's a no-op when the target already exists.
  • Bump version in _initDatabase() and register the migration in MigrationRegistry.all.

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.