Refactor and optimize zine thumbnail handling:#238
Merged
Conversation
- Restrict thumbnail refresh to projects with existing covers. - Adjust `RefreshStaleUnifiedThumbnailsJob` for efficiency. - Prevent unnecessary scan triggers on `repo_link` changes or clears. - Add targeted tests for thumbnail purge and scan workflows. - Streamline `ShipCheckService` and `UnifiedScreenshotFinder` to improve caching and configuration flexibility. - Sync `package-lock.json` to reflect updated peer and dev dependencies.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the unified thumbnail (“zine cover”) pipeline to avoid blind GitHub scans, add an owner-triggered refresh flow, and tighten caching so covers update when they exist without wasting work when they don’t.
Changes:
- Stop auto-enqueueing cover scans on project create/undiscard/repo_link edits; instead purge stale covers on repo changes and discover zines on demand / preflight / ship.
- Add owner-triggered cover refresh endpoints + frontend polling UI, with Rack::Attack throttling and Pundit gating.
- Improve finder caching semantics (
skip_nil,allow_representative-aware keys) and restrict stale-refresh sweeps to projects that already have an attached cover; update and extend tests/docs accordingly.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/models/project_test.rb | Updates expectations around repo_link changes (no scan; synchronous purge). |
| test/jobs/refresh_stale_unified_thumbnails_job_test.rb | Ensures the stale refresher only enqueues for projects that already have a cover. |
| test/jobs/compute_project_unified_thumbnail_job_test.rb | Aligns test stubs with updated finder signature and new enqueue behavior. |
| package-lock.json | Syncs dependency lockfile changes (Tailwind/oxide wasm subtree). |
| db/seeds.rb | Reworks dev seed demo data to use “realistic” repos/covers via the unified screenshot pipeline. |
| config/routes.rb | Adds refresh_cover and cover_status member routes for projects. |
| config/initializers/rack_attack.rb | Adds a throttle for owner-triggered cover refresh scans. |
| app/services/ship_checks/unified_screenshot_finder.rb | Adds cache keying by representative-allowance, skip_nil, and a force cache-bust option. |
| app/services/ship_checks/safe_http.rb | Prefers IPv4 for pinned-IP SSRF-safe HTTP to avoid IPv6 routing failures. |
| app/services/ship_check_service.rb | Adds return_context: option so callers can reuse SharedContext (preflight optimization). |
| app/policies/project_policy.rb | Introduces refresh_cover? authorization for the new owner-only flow. |
| app/models/project.rb | Replaces auto-scan callback with synchronous cover purge on repo_link changes. |
| app/jobs/ship_preflight_job.rb | Reuses preflight context to enqueue cover refresh when HasZinePage passes. |
| app/jobs/refresh_stale_unified_thumbnails_job.rb | Restricts refresh sweep to projects that already have a unified_thumbnail attached. |
| app/jobs/compute_project_unified_thumbnail_job.rb | Accepts source_url/force/allow_representative and adjusts concurrency key arity for new args. |
| app/frontend/pages/projects/show.tsx | Adds “Check for my zine” UI + polling state machine for cover refresh. |
| app/controllers/projects_controller.rb | Implements refresh_cover + cover_status JSON endpoints and exposes permission to the frontend. |
| agents-docs/arch-services-infra.md | Documents updated finder signature, caching, and force/representative behavior. |
| agents-docs/arch-projects-journals.md | Updates unified_thumbnail refresh contract documentation (needs a small correction). |
| agents-docs/arch-explore.md | Updates unified_thumbnail refresh contract documentation (needs a small correction). |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- db/seeds.rb: resolve + verify Content-Type before transcoding demo covers, so an unknown/blank type skips gracefully instead of raising KeyError and aborting db:seed (mirrors the cover job's guard). - RefreshStaleUnifiedThumbnailsJob: replace the IN-subquery cover filter with an INNER JOIN on the unified_thumbnail attachment so Postgres can use the (record_type, record_id, name) index prefix; give two exclusion tests a cover so the join doesn't mask the repo_link / discard filters they assert. Addresses Copilot review findings on PR #238. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RefreshStaleUnifiedThumbnailsJobfor efficiency.repo_linkchanges or clears.ShipCheckServiceandUnifiedScreenshotFinderto improve caching and configuration flexibility.package-lock.jsonto reflect updated peer and dev dependencies.