[CoreAI] Coordinate model acquisition

[metascroy]

Acquires a prepared Core AI model for a selected bundle: restore it from the persisted bookmark, or materialize the source and specialize it (runtime/coreai_load_coordinator.{h,mm}).

• acquire_bookmark_model restores from the persisted bookmark; on a miss it materializes the source, specializes it with the persistent default SDK cache and publishes the new bookmark atomically.
• Restore errors fail without a source fallback. Sources and SDK pins are retained across completion.
• The backend that uses this lands in the next PR.
• Tests:
  • An acquisition-only fake SDK loader (runtime/test/coreai_fake_loader.{h,mm}) runs the coordinator in coreai_host_test without Swift or CoreAI.
  • The acquisition suite calls acquire_bookmark_model directly: cold specialization, warm restores, restore errors and misses, bookmark-save failures, same-key versus independent-key contention across processes, and a crash after the SDK call.
• README: the "Asset storage" acquisition flow.

No changes outside backends/apple/coreai.

Stack: 6 of 9, based on #23391. Review only this PR's commit. Next: #23393.

Test plan: built locally for macOS 27.0; ctest -N lists coreai_host_test and coreai_swift_bridge_test, which the Core AI workflow runs on the macOS 27 runner. lintrunner is clean.

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/23392

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 1beeef8 with merge base 0b3d26d ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[metascroy]

@claude review this PR

[claude]

Claude finished @metascroy's task in 2m 41s —— View job

---

The coordinator logic looks correct, but I'd ask for changes: the specialize-error and empty-bookmark paths have no tests, the fake loader carries unused knobs, and a repeated bookmark-save failure leaks one SDK cache entry per load attempt.

Detailed review

PR Review: #23392

Summary

This PR adds acquire_bookmark_model. Under the per-key lock it restores the model from a persisted bookmark. If the bookmark is missing or the restore misses, it materializes the source, specializes it and publishes the new bookmark. I found no correctness bugs in the coordinator. The issues are test gaps, unused test scaffolding, and one design question about save failures.

Code Quality

• runtime/test/coreai_acquisition_fixture.mm:164,220: the acquisition crash child uses the flag --bookmark-crash. That falls inside the --bookmark- prefix that bookmark_child_mode claims (coreai_bookmark_fixture.mm:96). It only works because main runs acquisition_child_mode first. If the two dispatch calls in coreai_host_test.mm are ever reordered, bookmark_child_mode takes the flag, rejects it and the crash test fails in a confusing way. Renaming it to --acquisition-crash would match --acquisition-parallel. Fix this →
• runtime/test/coreai_acquisition_fixture.mm:228 and coreai_host_test.mm:16: set_acquisition_test_executable only forwards to set_bookmark_test_executable, and main calls both with argv[0]. Per CLAUDE.md, a trivial single-use helper like this should go. Moving the existing set_bookmark_test_executable(argv[0]) call above both child-mode dispatches does the same job.
• runtime/test/coreai_fake_loader.{h,mm}: nothing in this PR uses emptyBookmark, specializeError, evictError, onEvict, specializationBookmark or the evictModelWithBookmark body. The PR body calls this an "acquisition-only fake", so either test these knobs here (see Testing) or add them in the PR that needs them.

Testing

• Untested coordinator paths (runtime/coreai_load_coordinator.mm), even though the fake already has the knobs for some of them:
  • Specialization returns an SDK error (specializeError). This should give Internal and leave the bookmark untouched.
  • copyBookmarkData returns empty data (emptyBookmark). write_bookmark rejects that with InvalidExternalData (coreai_bookmarks.mm:231), and that path is never exercised.
  • The loader breaks its contract, for example a Hit with a nil model. This is the Invalid Core AI acquisition result check at coreai_load_coordinator.mm:46-49.
  • The loader == nil guard.
• Contention test is in-process only: the PR body says it tests "same-key versus independent-key contention across processes". In fact parallel_child runs two threads inside one child process (coreai_acquisition_fixture.mm:250-258). That checks that per-descriptor flock serializes threads, but nothing tests two processes specializing the same key at once. The existing bookmark lock test covers the lock primitive across processes, so this may be fine. If so, please fix the description; otherwise add a two-process variant.

Performance

• Repeated save failures leak SDK cache entries (runtime/coreai_load_coordinator.mm:90-91; needs discussion, low confidence): if write_bookmark fails after specialization, the new model is dropped and its SDK entry is orphaned. The next load has no bookmark, so it specializes again and creates another entry. CoreAIAcquisitionSaveFailureTest/Cold shows this: it expects specializations to be calls + 1. With a persistent failure such as a full disk or a read-only bookmarks directory, every load attempt adds one more untracked entry to the persistent SDK cache. The backend never reclaims these ("does not track historical or unrecorded SDK entries"). Two options:

  • Best-effort evictModelWithBookmark: on publish failure, after releasing the model's pin.
  • Return the model anyway, so the load succeeds and the next load retries publication.

  If the SDK de-duplicates specializations by source, this doesn't matter. Please confirm that either way.

Recommendation

Request Changes

Nothing here is a correctness bug in the coordinator. Before merging I'd ask for the missing error-path tests (specialize error, empty bookmark), the unused fake knobs to be removed or tested, and a decision on the save-failure leak.