Skip to content

feat(screenshots): import into the App Asset Library (--to-library) - #37

Merged
hanrw merged 3 commits into
mainfrom
feat/screenshots-import-to-library
Oct 11, 2026
Merged

hanrw merged 3 commits into
mainfrom
feat/screenshots-import-to-library

Conversation

@hanrw

@hanrw hanrw commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Why

Apple deprecated app screenshot sets in API 4.5.1. asc screenshots import, used by the screenshot editor and app-shots export, still uploads into them. This adds the App Asset Library path, opt-in for now.

What changed

asc screenshots import --version-id V --from export.zip --to-library [--existing fail|replace|append] [--placement-group G]... [--dry-run]

  • Upload: each distinct file (by SHA-256) goes to the app's asset library once, and asc waits for processing.
  • Slot: each image goes to the version-localization placement group that matches its exact pixel size, read from Apple's reference data. No hand-kept mapping table.
  • Order: one reorder per group, to match the manifest.
  • Existing placements: fail by default. The group is reported as conflict with a replace suggestion, and other groups continue. replace deletes the old placements first; append keeps them.
  • Results: one per locale and group, with status (planned / placed / conflict / ambiguous / noMatchingGroup / overLimit / failed) and next commands. The exit code is non-zero if any group isn't placed.
  • REST: POST /api/v1/versions/:versionId/screenshots/import?to-library=true, with the zip as the body.
  • Without --to-library: unchanged, plus a deprecation note on stderr. --existing, --placement-group and --dry-run are refused without --to-library, so a typo can't run a real import into sets.
  • Not yet supported: app previews, experiment treatments, iMessage screenshots, dedupe across runs, and making this the default.

Shared change: Affordance gains flags for bare CLI flags (--to-library). The four-argument initializer is kept so installed plugins still link. Because the struct's layout changed, rebuild the ASC Pro plugin after this merges.

How it was verified

  • Test-first: 12 planner, 10 execute and 8 result tests in Domain; 10 import and 3 file-reader tests that assert the full CLI and REST JSON. swift test passes: 651 + 1418 + 753.
  • Live --dry-run (read-only) on a version in Prepare for Submission, using a synthetic two-locale zip:
    • 1290x2796 → IPHONE_DYNAMIC_ISLAND_LARGE_PROFILE, planned.
    • 2048x2732 → ambiguous (IPAD_13_PROFILE, IPAD_129_PROFILE), exit code 1.
    • With --placement-group IPAD_13_PROFILE → both planned, exit code 0.
  • Live reference data: every image spec has min equal to max, so matching on exact size is safe. Only 3 of 74 sizes are ambiguous: iPad 12.9"/13" and Apple TV/Vision Pro.
  • A real import (upload → place → reorder) still needs a test app.

Checklist

  • A failing test came first; swift test passes
  • A command that returns data is also served by asc web-server
  • User-visible change: one line under ## [Unreleased] in CHANGELOG.md
  • make docs && make check-docs passes
  • No API key, .p8, issuer ID or Apple ID session in the diff, output or screenshots

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Import screenshot-export ZIPs directly into a version’s asset library, with screenshots matched to placement groups by size, locale, and order.
    • Preview planned imports with --dry-run, or choose how to handle existing screenshots: fail, replace, or append.
    • Use the REST API to import screenshot ZIPs into the asset library.
  • Updates
    • Imports without --to-library continue using screenshot sets; this workflow is deprecated.

`asc screenshots import --to-library` uploads each distinct file once to
the app's asset library, waits for processing, places each image in the
version-localization slot that matches its exact pixel size, and orders
each group to match the manifest. `--existing fail|replace|append`
decides what happens to a group that already has placements;
`--placement-group` settles the few sizes that fit two groups (iPad
12.9"/13", Apple TV/Vision Pro); `--dry-run` prints the plan. One result
per locale and group, with status and next commands. Also on REST:
POST /api/v1/versions/:versionId/screenshots/import?to-library=true.

Affordance gains `flags` for bare CLI flags; the four-argument
initializer is kept for installed plugins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 73d0bc56-614f-487a-89a1-98b54a24453a

📥 Commits

Reviewing files that changed from the base of the PR and between cdb570d and d60ce4d.


📒 Files selected for processing (2)
  • CHANGELOG.md
  • Sources/Domain/Shared/RESTPathResolver.swift

🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

The change adds asset-library screenshot imports from ZIP exports through CLI and REST paths. It plans placements by image size and locale, supports dry runs and existing-placement policies, and reports per-group outcomes. Imports without the library option continue through the screenshot-set workflow.

Changes

Screenshot asset-library import

Layer / File(s) Summary
Import data and result contracts
Sources/Domain/Apps/AssetLibrary/Import/ScreenshotImportGroupResult.swift, Sources/Domain/Apps/AssetLibrary/Import/ScreenshotImportPlan.swift, Sources/Domain/Shared/Affordance.swift, Sources/ASCCommand/Commands/Screenshots/ScreenshotImportFiles.swift, Tests/DomainTests/Apps/AssetLibrary/Import/ScreenshotImportGroupResultTests.swift, Tests/DomainTests/Shared/AffordanceTests.swift, Tests/ASCCommandTests/Commands/Screenshots/ScreenshotImportFilesTests.swift
Screenshot import files include image dimensions and a content hash. Group results define statuses, placement details, serialized output, and status-dependent links. CLI affordances can render valueless flags.
Placement planning and execution
Sources/Domain/Apps/AssetLibrary/Import/ScreenshotImportPlan.swift, Sources/Domain/Apps/AssetLibrary/AssetProcessingWait.swift, Sources/ASCCommand/Commands/AssetLibrary/AssetImagesCommand.swift, Tests/DomainTests/Apps/AssetLibrary/Import/ScreenshotImportPlanTests.swift, Tests/DomainTests/Apps/AssetLibrary/Import/ScreenshotImportExecuteTests.swift, Tests/DomainTests/TestHelpers/MockRepositoryFactory.swift
Planning matches screenshots to placement groups by size and order, applies existing-placement policies, and deduplicates uploads by content hash. Execution uploads files, waits for processing, creates placements, and reorders them. Tests cover planning, upload outcomes, and placement policies.
CLI import and ZIP processing
Sources/ASCCommand/Commands/Screenshots/ScreenshotsImport.swift, Tests/ASCCommandTests/Commands/Screenshots/ScreenshotsImportToLibraryTests.swift, CHANGELOG.md, docs/commands.md, docs/README.md, docs/features/asset-library/README.md, docs/features/screenshot-editor/README.md
The CLI validates library-only options and chooses between asset-library import and the existing screenshot-set path. It supports dry runs and reports failure when any result is unsuccessful. Documentation and tests describe the modes, options, and outcomes.
REST import endpoint and routing
Sources/ASCCommand/Commands/Web/Controllers/ScreenshotImportController.swift, Sources/ASCCommand/Commands/Web/RESTRoutes.swift, Sources/Domain/Apps/AssetLibrary/Import/ScreenshotImport+RESTRoutes.swift, Sources/Domain/Shared/RESTPathResolver.swift, Tests/ASCCommandTests/Commands/Screenshots/ScreenshotsImportToLibraryTests.swift, docs/features/screenshot-editor/README.md
The REST endpoint validates query options, accepts ZIP uploads up to 500 MiB, runs the import, and removes extracted files. Route registration adds the endpoint when its required repositories are available.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ScreenshotsImport
  participant ScreenshotImportPlan
  participant imageRepo
  participant placementRepo
  ScreenshotsImport->>ScreenshotImportPlan: Plan screenshot placements
  ScreenshotImportPlan->>imageRepo: Upload unique files and check processing
  ScreenshotImportPlan->>placementRepo: Create and reorder placements
  ScreenshotImportPlan-->>ScreenshotsImport: Return per-group results
Loading

Merge Risk: ⚪ Minimal · up to d60ce

The screenshot-import REST path is wired into normal initialization. Although its registration also depends on video-repository setup, the available evidence does not establish a concrete failure condition that would make the endpoint unavailable; no merge-blocking risk is identified.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 26.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 46 functions across 21 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the primary change: adding an opt-in --to-library path for screenshot imports.
Description check Passed The description includes all required sections, explains the motivation and implementation, documents verification, and completes the repository checklist. The issue link remains unspecified in `Fixes…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 26.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 46 functions across 21 files. (1 skipped: 1 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR




🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.42373% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.29%. Comparing base (600c1c6) to head (d60ce4d).

Files with missing lines Patch % Lines
...mmand/Commands/Screenshots/ScreenshotsImport.swift 62.90% 23 Missing ⚠️
...etLibrary/Import/ScreenshotImportGroupResult.swift 74.60% 16 Missing ⚠️
...pps/AssetLibrary/Import/ScreenshotImportPlan.swift 98.55% 2 Missing ⚠️
...d/Commands/Screenshots/ScreenshotImportFiles.swift 92.85% 1 Missing ⚠️
Sources/Domain/Shared/RESTPathResolver.swift 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #37      +/-   ##
==========================================
- Coverage   84.43%   84.29%   -0.15%     
==========================================
  Files         519      524       +5     
  Lines       15600    15879     +279     
==========================================
+ Hits        13172    13385     +213     
- Misses       2428     2494      +66     
Files with missing lines Coverage Δ
...and/Commands/AssetLibrary/AssetImagesCommand.swift 66.66% <ø> (-5.68%) ⬇️
...Domain/Apps/AssetLibrary/AssetProcessingWait.swift 100.00% <100.00%> (ø)
...etLibrary/Import/ScreenshotImport+RESTRoutes.swift 100.00% <100.00%> (ø)
Sources/Domain/Shared/Affordance.swift 100.00% <100.00%> (ø)
...d/Commands/Screenshots/ScreenshotImportFiles.swift 92.85% <92.85%> (ø)
Sources/Domain/Shared/RESTPathResolver.swift 72.72% <0.00%> (-27.28%) ⬇️
...pps/AssetLibrary/Import/ScreenshotImportPlan.swift 98.55% <98.55%> (ø)
...etLibrary/Import/ScreenshotImportGroupResult.swift 74.60% <74.60%> (ø)
...mmand/Commands/Screenshots/ScreenshotsImport.swift 62.93% <62.90%> (+6.26%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hanrw
hanrw merged commit f1b096b into main Oct 11, 2026
5 checks passed
@hanrw
hanrw deleted the feat/screenshots-import-to-library branch October 11, 2026 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant