Repository navigation
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/23387
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (3 Unrelated Failures)As of commit 57fedba with merge base 61ec450 ( FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
0485975 to
8bdd47d
Compare
|
@claude review this PR |
|
Claude finished @metascroy's task in 3m 9s —— View job This PR adds the Core AI manifest parser, AOT asset selection, a host GoogleTest target and CI wiring. The parser logic looks correct against the exporter in Detailed reviewSummaryThe PR adds Code Quality
Infrastructure
Testing
RecommendationApprove once the README and the unused define are fixed. The test gaps and the CI path filter are worth handling here or in the next stack PR; none of them is a correctness bug in the parser. |
Summary:
Add the native parser the Core AI runtime uses to read the delegate manifest,
and build and run the backend's host tests in the Core AI workflow.
- Runtime (`runtime/coreai_assets.{h,mm}`): parse and validate the manifest
for `inline` source and `aot_compiled_inline` assets, including the ordered
I/O names, file sizes and bundle digests, and select AOT assets by exact
platform and Core AI architecture match with no fallback.
- Tests: a Foundation-only `coreai_host_test` GoogleTest executable (no Swift,
no Core AI SDK) for the parser and asset selection. Path and file-map checks
are covered because they feed filesystem writes; the manifest is otherwise
treated as trusted exporter output.
- README: the "Runtime" manifest description, AOT architecture selection and
"Host Tests".
Changes outside `backends/apple/coreai` are CMake and CI only, and nothing
changes unless `EXECUTORCH_BUILD_COREAI=ON`:
- `tools/cmake/preset/default.cmake`: add `EXECUTORCH_BUILD_COREAI`
(default OFF).
- `CMakeLists.txt`: add the backend subdirectory under that option. The host
test is registered inside the backend when `EXECUTORCH_BUILD_TESTS=ON`.
- `.github/workflows/coreai.yml`: build the backend directory for macOS 27 and
run every CTest test it registers.
Test Plan:
```
# Local: configure and build only
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release \
-DCMAKE_OSX_DEPLOYMENT_TARGET=27.0 -DEXECUTORCH_BUILD_COREAI=ON \
-DEXECUTORCH_BUILD_TESTS=ON -DEXECUTORCH_BUILD_EXTENSION_DATA_LOADER=ON
cmake --build build --target backends/apple/coreai/all
# CI (macOS 27 runner, .github/workflows/coreai.yml)
ctest --test-dir build/backends/apple/coreai --output-on-failure --no-tests=error
```
CTest runs `coreai_host_test`.
Align the parser-only PR with its actual supported behavior and cover malformed manifest metadata before later storage and loading work depends on it. Test Plan: Built and ran the native host suite on macOS 27: all 24 tests passed. CMake platform/test gating and workflow formatting-config checks passed. The Python suite was blocked during collection by the installed PyTorch/torchao compatibility error. Authored with assistance from Devmate (OpenAI GPT-6-Astra).
8bdd47d to
5f68c11
Compare
|
@claude review this code Updated head: |
|
Claude finished @metascroy's task in 3m 47s —— View job This PR adds the Core AI manifest parser, AOT asset selection, a Foundation-only host test, and CI wiring. Every finding from the previous review is fixed at Detailed reviewSummaryThis PR parses and validates Previous findings, all resolved:
Not run: this runner is Linux, so I could not build or run anything. The diff was taken against base commit Code Quality
Infrastructure
Testing
RecommendationApprove. None of these blocks merge. The returned-field test and the two |
Summary: Reject manifests whose asset names collide on case-insensitive, Unicode-normalizing filesystems before materialization. Handle missing input safely, verify returned metadata and ordering, and enforce Objective-C++ formatting in backend CI. Test Plan: The new filesystem-alias regressions failed before the fix. All 32 native parser tests and all 113 CoreAI Python tests passed afterward, covering source/AOT aliases, valid Unicode names, nil input, returned fields, name ordering, and version parsing. Formatting, workflow syntax, and whitespace checks passed.
Adds the native parser the Core AI runtime uses to read the delegate manifest, and builds and runs the backend's host tests in the Core AI workflow. This is the first of the runtime PRs; it does not add an ExecuTorch delegate yet.
runtime/coreai_assets.{h,mm}):inlinesource andaot_compiled_inlineassets:function, orderedinput_names/output_names, thefilessize map andbundle_digests.coreai_host_testGoogleTest executable (no Swift, no Core AI SDK) for the parser and asset selection. Path and file-map checks are covered because they feed filesystem writes; the manifest is otherwise treated as trusted exporter output.backends/apple/coreai/.clang-format: the root style without itsLanguage: Cppkey, so clang-format also formats the backend's Objective-C headers.backends/apple/coreaiare CMake and CI only, and nothing changes unlessEXECUTORCH_BUILD_COREAI=ON:tools/cmake/preset/default.cmake: addEXECUTORCH_BUILD_COREAI(default OFF).CMakeLists.txt: add the backend subdirectory under that option..github/workflows/coreai.yml: buildbackends/apple/coreai/allfor macOS 27 with tests on, and run every CTest test the backend registers (--no-tests=error).Stack: 1 of 9 Core AI runtime PRs, based on
main. Next: #23388.Test plan: built locally for macOS 27.0 (
backends/apple/coreai/all,EXECUTORCH_BUILD_TESTS=ON);ctest -Nlistscoreai_host_test, which the Core AI workflow runs on the macOS 27 runner.lintrunneris clean.