Skip to content

Support metadata-validated POD WinRT vectors - #179

Open
leileizhang (lei9444) wants to merge 5 commits into
mainfrom
lei9444-post-release-pod-vectors
Open

leileizhang (lei9444) wants to merge 5 commits into
mainfrom
lei9444-post-release-pod-vectors

Conversation

@lei9444

@lei9444 leileizhang (lei9444) commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Partially addresses #161: phase-one support for empty and populated checked POD WinRT vectors, not completion of the broader struct-collection issue.

  • Validate closed metadata types, exact element identity/IIDs, and recursive natural POD layouts before publication; reject owned or incomplete layouts.
  • Add aligned owned POD storage and cache immutable vtable plans using the existing central libffi callback machinery. Cover IVector<T>, live/snapshot IVectorView<T>, IIterable<T>, iterators, bulk operations, and observable/reentrant lifetime paths, including ARM64 HFAs and larger/nested aggregates.
  • Preserve word-sized scalar/reference/HSTRING fast paths, aliases, field-wise equality (padding ignored, signed zeros equal, NaNs unequal), ownership, and null distinctions. Map admission, metadata-free unsafe-constructor contracts, owned structs, and top-level scalar limits remain unchanged.

No public facade, codegen implementation, dependency, or release-workflow changes. The existing native_callback implementation is reused unchanged. See docs/architecture/winrt-pod-vectors.md for the design and boundaries.

Local validation and revision attribution

Implementation validated at 0ef52e6. Follow-up bb153fb changes only release wording in four documentation files; git diff --check passed. The original three implementation/coverage commits were not rebased or rewritten, and historical evidence manifests remain unchanged.

Validation at the implementation revision Result
Native ARM64 cargo test --locked --offline -p dynwinrt 503 unit + 17 metadata-lifetime + 1 WinRT regression + 4 doctests passed; 1 existing WinAppSDK initialization ignore
Focused collection/map/callback tests, ARM64 optimized 63 passed
Same focused tests, Windows x64 / i686 emulation on ARM64 66 / 63 passed; not native x64/i686 hardware qualification
Selected codegen snapshot/collection/observable/preparation tests 15 passed, not the full codegen suite
Matched Python WinRT/runtime regressions 92 passed
Matched JS scoped collection/root tests and generated/native-class regressions 10 + 21 passed; 1 existing x64-only map-test skip
Public JS typechecks and existing TS runner tests Passed; 2 runner tests

Matched generated JS/Python E2E passed stock InkStrokeBuilder Point inputs and all three Geopath/GeoboundingBox BasicGeoposition overloads. The optional pod_hint_specs.json fixture exercises generated RectInt32 vector preparation only and remains explicitly documented as such. Empty producers remain real non-null collections; native consumers retain their own empty-input semantics.

Additional local ARM64 gallery trial

A later user-confirmed trial used the exact release addon from 0ef52e69 (SHA256 D214D97ACB6BFBB61DA14F6871138C325E93F5754DBC84F4BEB8AF55F7EB0F94) with unchanged f50fd14 codegen, all 132 WinMD hashes, and all 1,313 generated files. Real empty RectInt32 input matched the prior null control (850x566, 481,100 mask bytes; 401,288 opaque / 79,812 transparent). A nonempty rectangle (225, 20, 400, 540) passed field roundtrip and native AI mask execution, and the renderer → IPC → utility → native → canvas route also passed. Missing-image input still rejected with 0x80070002.

This is local evidence for those gallery paths, not general AI/UI or WinAppSDK qualification; it does not change the preparation-only test fixture's scope.

Release admission

Proposed for this release, subject to PR review, final integration, and platform CI; not an already-released capability or a claim that CI is green. A local merge-tree check against main at 6a9bb4ebf57b3a9cd81891decc3f7701ee08e56b was conflict-free, but the merge result has not been requalified by the historical local suites.

Coverage follow-up: 7da277c

The failed coverage run reached the final Python aggregate gate: 7,220 / 10,340 lines = 69.83%, below the unchanged 70% minimum. It was not a build or test-assertion failure. Adding the POD E2E inputs expanded the measured generated projection set; a diagnostic control without those two new specs measured 71.06%.

7da277c changes only the existing Python geolocation E2E case: constructor/factory parity, BasicGeoposition view bulk reads and lookup, shape/spatial/altitude properties, and reconstructed bounding-box checks. No production code, workflows, coverage filters, thresholds, or existing assertions were changed.

The local Python coverage slice used the official codegen artifact for CI merge 5215faa, source-matched native runtime, and local SDK 26100 metadata. The before result matched CI's numerator and denominator exactly; after the assertions it measured 7,263 / 10,348 = 70.19%, passing the unchanged validator. All 44 standard Python E2E specs and 4 + 19 implementation scenarios passed, as did binding tests (14 existing unavailable WinUI-fixture skips), the focused two POD specs, threshold regression tests, Python compilation, and git diff --check.

This reproduction was native ARM64 / CPython 3.13.15 / cached coverage.py 7.16.0; the failed hosted run used x64 / CPython 3.12.10 / coverage.py 7.16.1. No new remote CI success is claimed; the follow-up still requires its normal CI checks.

Retain word/reference fast paths and map admission; add aligned POD ownership, cached metadata-driven collection callbacks, and SDK-typed ABI/lifetime coverage. This is the vector-only first phase of #161, isolated from the current release.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Exercise SDK Point and BasicGeoposition collection consumers plus optional AI hint RectInt32 vector preparation with matching JS/Python runtimes. Keep map, non-POD, scalar, unsafe-constructor and current-release boundaries explicit.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Hold the owner before retaining input values and until notification/state borrows return. Verify final reentrant Release also drops handler closures without creating owner cycles.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace historical post-release exclusion wording with phase-one scope proposed for release, subject to PR review, final integration and platform CI. Keep implementation and historical validation attribution unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Mixed-language test coverage

Workflow status: ✅ Passed

Layer Lines Functions Branches/regions
Rust, including native .pyd/.node 86.41% 81.54% 86.07% regions
Python aggregate 70.19% n/a 36.95% branches
Python runtime 99.12% n/a 97.37% branches
Generated Python WinRT projections 68.99% n/a 27.3% branches
Generated Python WinRT implementations 72.3% n/a 47.04% branches
JavaScript aggregate 21.98% 24.37% 57.89% branches
JavaScript runtime 44.27% 45.76% 78.99% branches
Generated WinRT projections 22.55% 18.39% 55.34% branches
Generated WinRT implementations 48.66% 59.76% 61.14% branches
Generated Classic COM projections 11.92% 23.97% 54.2% branches

View workflow run and download full HTML/LCOV/XML reports

Exercise constructor/factory parity, typed BasicGeoposition bulk lookup, and geoshape properties in the existing native E2E case. Restore the unchanged Python aggregate coverage gate after the added POD projection closure lowered coverage to 69.83%; matched local Python coverage now reaches 70.19%. No production or threshold changes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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