fix(collections): make per-card remove buttons usable on touch #19
Reference in New Issue
Block a user
Delete Branch "fix/collection-remove-touch"
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?
The per-card remove buttons on a collection were revealed only on
hover/focus-within(opacity: 0 → 1), so on touch they were invisible and undiscoverable.Show them persistently at the 640px breakpoint and enlarge the hit area from 24px to 32px (kept below the full 44px floor because the button floats over a 4-up cover thumbnail — a touch-floor square would swallow the cover).
Testing
CSS-contract test (jsdom can't evaluate
@media): the mobile block makes.removeopaque — red before, green after. svelte-check clean.Note / version
Touches
collections/[id]like #15, but a different region (the.removecard overlay, not the title-row icon buttons #15 converted) — should merge cleanly.fix:→ 0.88.1 from main.🤖 Generated with Claude Code
Two high findings sharing one root cause: long-running daemon work that ignored the shutdown signal until it finished naturally. **Cron tick (crawler).** `CronContext::run_tick` ran the metadata-pass body inside `AssertUnwindSafe(...).catch_unwind().await` with no cancellation. A fresh-catalog walk (many minutes) wedged `DaemonHandle::shutdown`, `Supervisors::reload_crawler` (under the supervisor lock), and SIGTERM responsiveness for the whole pass. Wrap the body in `tokio::select! { biased; _ = self.cancel.cancelled() => ...; r = body => ... }`. On cancel the body future drops, which cascades to dropping `metadata.run()` — exactly what gives Chromium/lease teardown a chance via the BrowserManager idle reaper. The advisory unlock + conn drop still run unconditionally. **Analysis dispatch.** `process_lease` wrapped the vision call only in `tokio::time::timeout(self.job_timeout, …)` — default 600s. Toggling analysis off in the dashboard (under the supervisor lock) or stopping the container blocked on whichever call was in flight. Same fix: `tokio::select!` against `self.cancel.cancelled()`. On cancel, `jobs::release` returns the lease to the queue without burning an attempt — next tick picks it up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>`VisionClient::analyze` ran `image::load_from_memory`, `DynamicImage::resize_exact(Triangle)`, and per-band JPEG encodes directly on the tokio task. Each call is hundreds of ms of pure CPU per page. With multiple workers, the runtime threads got starved — axum handlers, SSE streams, the crawler daemon's async timers, and other tasks sharing the runtime all stalled while analysis was active. Extract a `prepare_analysis` helper that does ALL the CPU work (decode + plan + width-reduce + slice + JPEG encode) and returns a `PreparedAnalysis { Single | Sliced | Undecodable }` of already-encoded byte payloads. `analyze` calls it inside `tokio::task::spawn_blocking`, then the async HTTP loop only iterates over the prepared bytes — no image-crate operations happen on the runtime any more. Single page → one spawn_blocking → one HTTP call. Tall page → one spawn_blocking → N OCR calls + 1 grounding call. Memory peak rises slightly for the Sliced path (all bands held encoded at the same time instead of one-at-a-time) — for a typical manga page split into 4 slices at ~200 KB each, that's ~600 KB of extra peak. Negligible vs. the runtime-starvation cost. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Two ship-blockers from the rereview: 1. Env var name mismatch: 0.87.12 renamed every operator surface to ANALYSIS_VISION_MODEL but the reader still called env::var on ANALYSIS_MODEL — every documented deploy ran analysis with an empty model. Rename the reader. 2. Compose regression test was substring-only and missed exactly the ANALYSIS_VISION_MODEL vs ANALYSIS_MODEL RHS mismatch from 0.87.12. Tightened to require each backend-consumed var either be in a small hardcoded allowlist (BIND_ADDRESS, STORAGE_DIR) or have its RHS interpolate `${KEY...}` with the same name. Mutation-confirmed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>0.87.14 followup. The 0.87.14 test used MissedTickBehavior::Burst — any post-prep yield replayed missed ticks and could fake the >= 2 threshold. Switch to Skip and assert on `ticks_post - ticks_pre >= 50`, which sits well above the inlined-prep ceiling (~5) and far below the spawn_blocking floor (~700+). Mutation-confirmed. Also corrects: prepare_analysis doc count ("five fallback paths" → 4). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>0.87.12 added a regression test for backend env wiring, but its BACKEND_CONSUMED list was hand-maintained and drifted: ~22 vars read by backend/src/ never made it into compose's backend.environment. An operator setting ANALYSIS_TEMPERATURE=0.3 in .env got a silent no-op — exactly the bug class the test was supposed to catch. Make .env.example the single source of truth: * docker_compose_wires_every_documented_backend_env_var now reads .env.example at test time, filters NOT_BACKEND_CONSUMED_IN_ENV_EXAMPLE (compose-side composed vars, frontend-only vars, sidecar vars), and requires every remaining key to be wired into compose with a matching ${KEY...} interpolation OR allowlisted via COMPOSE_RHS_EXEMPT. * every_backend_src_env_read_is_documented_or_allowlisted scans backend/src/ for env::var / env_bool / env_i64 / ... reads and requires each key to be in .env.example OR in INTERNAL_NOT_CONFIGURABLE with a per-entry rationale. Catches the silent-no-op bug from the other side. Wire the previously-missing env vars into docker-compose.yml's backend.environment (CRAWLER_DAEMON, CRAWLER_DAILY_AT, CRAWLER_TZ, CRAWLER_IDLE_TIMEOUT_S, CRAWLER_CHAPTER_WORKERS, CRAWLER_JOB_RETENTION_DAYS, CRAWL_METRICS_RETENTION_DAYS, CRAWLER_RATE_MS, CRAWLER_CDN_RATE_MS, CRAWLER_USER_AGENT, CRAWLER_PHPSESSID, CRAWLER_COOKIE_DOMAIN, CRAWLER_TOR_CONTROL_COOKIE_PATH, all ANALYSIS_* tuning knobs). Document each new var in .env.example with the same default value shown in compose's `${KEY:-default}` form. Allowlist internal/system vars (HOME = chromium cache dir fallback, CRAWLER_BROWSER_*, CRAWLER_SKIP_*, the multi-paragraph ANALYSIS_*_PROMPT seeds) with per-entry rationale. Mutation-tested both sides: adding `env::var("MANGALORD_FAKE_NEW_KNOB")` to main.rs fails the scanner; removing the new compose wiring fails the documented-var test with all 23 missing keys named. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>0.87.20 narrowed but did not close the force-analyze race: ack_done / ack_failed / renew matched only on (id, state='running'), so a worker A whose lease was released mid-flight (force-analyze, reclaim_orphaned) could still come back later — once a successor B re-leased the same id and put it back to state='running' — and clobber B's lease. A's stale result became the row's `done` payload; B's in-flight work was silently lost. The pre-existing test even documented this hole: "We can't make ack_done's lease_id distinguish A from B today". Add `lease_generation BIGINT NOT NULL DEFAULT 0` to crawler_jobs (migration 0032). It is bumped by every lease, release / release_unowned, and reclaim_orphaned, so lease identity is `(id, generation)` rather than just `id`. Each Lease struct carries its generation. ack_done / ack_failed / renew / release match on `(id, generation, state='running')` so a stale ack from a prior generation finds zero rows and falls back to the existing warn-and-skip branch. Split the release surface in two: * release(lease_id, lease_generation) — owner-aware path for workers (graceful shutdown, session-expired), matches the caller's specific lease. * release_unowned(lease_id) — id-only path for force-analyze and ops tools that drop a running lease without holding a Lease struct; unconditionally bumps generation so any pending ack from the in-flight original becomes a no-op. Force-analyze in repo::page_analysis now uses release_unowned; the test in api_admin_analysis still passes unchanged (it only asserts the steady-state outcome). The pre-existing `ack_done_no_ops_when_ lease_was_stolen` test is strengthened: it now mints both A and B leases, asserts their generations differ, and verifies A's stale ack does NOT clobber B's running row — the assertion the old test explicitly couldn't make. Five new tests in tests/crawler_jobs.rs pin every facet: * lease_assigns_strictly_increasing_generation_per_release * ack_done_from_dead_lease_after_release_unowned_is_a_no_op * ack_failed_from_dead_lease_after_release_unowned_is_a_no_op * renew_from_dead_lease_is_a_no_op * release_unowned_bumps_generation_even_when_lease_is_held Mutation-confirmed: relaxing the ack_done guard to `lease_generation >= $2` (bind 0) makes the race test fail with state=done — the exact prior-bug shape. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>Pull request closed