morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #5228] [MERGED] fix(apple): classify keyed-archive objects by declared superclasses #6827

New issue

Closed

opened 2026-11-01 21:43:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:43:19 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5228
Author: @mhamzasakhi
Created: 10/19/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/uikit-view-controller-kinds

📝 Commits (4)

96e3e73 fix(apple): classify keyed-archive objects by declared superclasses
9779993 test(apple): cover each declared keyed-archive base with neutral class names
efa1d58 perf(apple): parse each keyed-archive class chain once
8785c67 Merge origin/main into fix/uikit-view-controller-kinds

📊 Changes

2 files changed (+127 additions, -18 deletions)

View changed files

📝 src/domain/apple/interfaceBuilderKeyedArchive.ts (+30 -2)
📝 tests/acceptance/applications/interfaceBuilderUIKitKeyedArchive.test.ts (+97 -16)

📄 Description

Summary

Fixes #5218. decode-interface-builder returned kind: "control" for
UINavigationController, UITabBarController and UISearchController in
NSKeyedArchiver nibs. These classes are UIViewController subclasses, and the
archive records that in each class descriptor's $classes chain.

Problem and expected behavior

classifyObject used class-name substrings only. These three names do not
contain viewcontroller, but they do contain control, so the control branch
matched. The archive already declares the superclass chain the decoder needs:
$classes: [UINavigationController, UIViewController, UIResponder, NSObject].

Expected: objects whose declared chain includes UIViewController return
view_controller and keep their class_name. UIButton stays control.

Change and scope

For keyed archives, classifyObject checks the declared $classes chain first,
most specific base first:
- UIViewController, NSViewController or NSWindowController → view_controller
- UIControl or NSControl → control
- UIView, NSView, UIWindow or NSWindow → view
When the chain names none of these bases, the existing name heuristic applies
unchanged. Constraints, layout guides, placeholders and resources such as
NSImage still classify as before.
$classes parsing now has one owner in the keyed-archive table (classChain).
It parses each descriptor once per archive UID into a Set, and NSArray
detection uses it too, replacing a duplicate parse.
Not included: NIBArchive and ibtool-dictionary inputs do not record a superclass
chain, so they keep the name heuristic. A customClass named like
UINavigationController in those formats is still classified by name. That is
a remaining limitation, not addressed here.

Contribution scope: an ordinary static format-decoding fix within existing
boundaries. A maintainer scope decision is not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: src/domain/apple/interfaceBuilderKeyedArchive.ts, keyed-archive object classification.
Contribution scope fit and linked maintainer decision: in scope (static format inspection); not applicable.
CLI and MCP/tool-catalog contract: no schema change. Only kind values change, within the existing enum.
Provider, bridge, target-format, or platform compatibility: keyed archives only. NIBArchive and ibtool dictionaries are unchanged.
Evidence, artifact, provenance, or reconstruction contract: graph node kind becomes more accurate for keyed archives. Beyond the reported classes, a keyed-archive object whose chain declares a UIKit/AppKit view base now returns view instead of a name-derived kind. For example, UIImageView was resource and UILabel was other.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: tests/acceptance/applications/interfaceBuilderUIKitKeyedArchive.test.ts adds a CLI acceptance case. A keyed archive with fifteen classes asserts each (class_name, kind) pair and complete object coverage:
- the three reported classes
- UIViewController
- UIButton
- UIImageView
- NSLayoutConstraint
- neutral-named classes that classify only through their declared UIKit or AppKit base:
  - SettingsScreen, InspectorPane, DocumentShell (view controllers)
  - RatingPicker, LevelGauge (controls)
  - CanvasSurface, AppShell, Palette (views)
The existing outlet test now shares the app-bundle setup helper.
Base reproduction (executed): with only the source change reverted, the new test fails on the kind assertion. With the change, it passes. The test also fails under each of these source mutations:
- removing NSControl
- removing NSViewController and NSWindowController
- removing NSView and NSWindow
- checking view before control
Performance (executed, review follow-up): a 2.1 MB keyed archive with 20,000 objects sharing one descriptor with a 20,000-name chain. Each figure is from 3 runs of the compiled CLI.

Revision Elapsed

Uncached first revision 15.7–16.5 s

Cached 3.4–4.1 s

main 3.5–4.6 s

Peak RSS is about 1.1 GB in all three; this is pre-existing. There is no timing regression test, because it would be flaky.
Remaining proof gaps: the archives are synthetic, not produced by Xcode ibtool. The macOS-only Interface Builder tests (nibNativeHierarchy, assetCatalogResourceKeys, and the darwin/ibtool cases in interfaceBuilderBundle) were skipped on Linux and did not run. By source inspection, their only kind assertion targets NSImage (resource), whose chain declares no UI base.
Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Revision	Elapsed
Uncached first revision	15.7–16.5 s
Cached	3.4–4.1 s
`main`	3.5–4.6 s

Validation performed

Run on Ubuntu 24.04 x64 with Node 24.18.0 and npm 11.16.0:

npm run test:focused -- tests/acceptance/applications/interfaceBuilderUIKitKeyedArchive.test.ts: 2 passed. With the source change reverted, and under each mutation above, the new case failed.
After the cache change, npm run test:focused -- with 8 Interface Builder acceptance, boundary and graph test files: 79 passed, 24 skipped (macOS/ibtool only), 0 failed.
npm run test:focused -- with the Interface Builder acceptance, boundary and native-graph tests (12 files: the IB acceptance tests, nibNativeHierarchy, interfaceBuilderBundle, interfaceBuilderGraph, nativeInvestigationGraph, EnhancedNativeInvestigation, AppleAssetCatalogAnalysis): 117 passed, 37 skipped (macOS/ibtool only), 0 failed.
npm run check:fast: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. Kind values stay within the existing enum. Callers that relied on the previous incorrect kinds for keyed-archive view-controller, control or view subclasses will see corrected values.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (no provider). macOS/Xcode lanes not run.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new reads, execution, or network access.

Author readiness

Human owner: @mhamzasakhi

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5228 **Author:** [@mhamzasakhi](https://github.com/mhamzasakhi) **Created:** 10/19/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/uikit-view-controller-kinds` --- ### 📝 Commits (4) - [`96e3e73`](https://github.com/morluto/rea/commit/96e3e73c5fdb63ba7edc29f71293c73b644cac62) fix(apple): classify keyed-archive objects by declared superclasses - [`9779993`](https://github.com/morluto/rea/commit/97799930e85969f8701c42be4c88cdadf84a882f) test(apple): cover each declared keyed-archive base with neutral class names - [`efa1d58`](https://github.com/morluto/rea/commit/efa1d58f4c93a66932d79a13f850f0c5a2d7ee9d) perf(apple): parse each keyed-archive class chain once - [`8785c67`](https://github.com/morluto/rea/commit/8785c6758b353ca31228b81a2a1e6cba0730a442) Merge origin/main into fix/uikit-view-controller-kinds ### 📊 Changes **2 files changed** (+127 additions, -18 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/apple/interfaceBuilderKeyedArchive.ts` (+30 -2) 📝 `tests/acceptance/applications/interfaceBuilderUIKitKeyedArchive.test.ts` (+97 -16) </details> ### 📄 Description ## Summary Fixes #5218. `decode-interface-builder` returned `kind: "control"` for `UINavigationController`, `UITabBarController` and `UISearchController` in NSKeyedArchiver nibs. These classes are `UIViewController` subclasses, and the archive records that in each class descriptor's `$classes` chain. ## Problem and expected behavior `classifyObject` used class-name substrings only. These three names do not contain `viewcontroller`, but they do contain `control`, so the control branch matched. The archive already declares the superclass chain the decoder needs: `$classes: [UINavigationController, UIViewController, UIResponder, NSObject]`. Expected: objects whose declared chain includes `UIViewController` return `view_controller` and keep their `class_name`. `UIButton` stays `control`. ## Change and scope - For keyed archives, `classifyObject` checks the declared `$classes` chain first, most specific base first: - `UIViewController`, `NSViewController` or `NSWindowController` → `view_controller` - `UIControl` or `NSControl` → `control` - `UIView`, `NSView`, `UIWindow` or `NSWindow` → `view` - When the chain names none of these bases, the existing name heuristic applies unchanged. Constraints, layout guides, placeholders and resources such as `NSImage` still classify as before. - `$classes` parsing now has one owner in the keyed-archive table (`classChain`). It parses each descriptor once per archive UID into a `Set`, and `NSArray` detection uses it too, replacing a duplicate parse. - Not included: NIBArchive and ibtool-dictionary inputs do not record a superclass chain, so they keep the name heuristic. A `customClass` named like `UINavigationController` in those formats is still classified by name. That is a remaining limitation, not addressed here. Contribution scope: an ordinary static format-decoding fix within existing boundaries. A maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/domain/apple/interfaceBuilderKeyedArchive.ts`, keyed-archive object classification. - Contribution scope fit and linked maintainer decision: in scope (static format inspection); not applicable. - CLI and MCP/tool-catalog contract: no schema change. Only `kind` values change, within the existing enum. - Provider, bridge, target-format, or platform compatibility: keyed archives only. NIBArchive and ibtool dictionaries are unchanged. - Evidence, artifact, provenance, or reconstruction contract: graph node `kind` becomes more accurate for keyed archives. Beyond the reported classes, a keyed-archive object whose chain declares a UIKit/AppKit view base now returns `view` instead of a name-derived kind. For example, `UIImageView` was `resource` and `UILabel` was `other`. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `tests/acceptance/applications/interfaceBuilderUIKitKeyedArchive.test.ts` adds a CLI acceptance case. A keyed archive with fifteen classes asserts each `(class_name, kind)` pair and complete object coverage: - the three reported classes - `UIViewController` - `UIButton` - `UIImageView` - `NSLayoutConstraint` - neutral-named classes that classify only through their declared UIKit or AppKit base: - `SettingsScreen`, `InspectorPane`, `DocumentShell` (view controllers) - `RatingPicker`, `LevelGauge` (controls) - `CanvasSurface`, `AppShell`, `Palette` (views) The existing outlet test now shares the app-bundle setup helper. - Base reproduction (executed): with only the source change reverted, the new test fails on the kind assertion. With the change, it passes. The test also fails under each of these source mutations: - removing `NSControl` - removing `NSViewController` and `NSWindowController` - removing `NSView` and `NSWindow` - checking `view` before `control` - Performance (executed, review follow-up): a 2.1 MB keyed archive with 20,000 objects sharing one descriptor with a 20,000-name chain. Each figure is from 3 runs of the compiled CLI. | Revision | Elapsed | | --- | --- | | Uncached first revision | 15.7–16.5 s | | Cached | 3.4–4.1 s | | `main` | 3.5–4.6 s | Peak RSS is about 1.1 GB in all three; this is pre-existing. There is no timing regression test, because it would be flaky. - Remaining proof gaps: the archives are synthetic, not produced by Xcode `ibtool`. The macOS-only Interface Builder tests (`nibNativeHierarchy`, `assetCatalogResourceKeys`, and the darwin/ibtool cases in `interfaceBuilderBundle`) were skipped on Linux and did not run. By source inspection, their only kind assertion targets `NSImage` (`resource`), whose chain declares no UI base. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Run on Ubuntu 24.04 x64 with Node 24.18.0 and npm 11.16.0: - `npm run test:focused -- tests/acceptance/applications/interfaceBuilderUIKitKeyedArchive.test.ts`: 2 passed. With the source change reverted, and under each mutation above, the new case failed. - After the cache change, `npm run test:focused --` with 8 Interface Builder acceptance, boundary and graph test files: 79 passed, 24 skipped (macOS/ibtool only), 0 failed. - `npm run test:focused --` with the Interface Builder acceptance, boundary and native-graph tests (12 files: the IB acceptance tests, `nibNativeHierarchy`, `interfaceBuilderBundle`, `interfaceBuilderGraph`, `nativeInvestigationGraph`, `EnhancedNativeInvestigation`, `AppleAssetCatalogAnalysis`): 117 passed, 37 skipped (macOS/ibtool only), 0 failed. - `npm run check:fast`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Kind values stay within the existing enum. Callers that relied on the previous incorrect kinds for keyed-archive view-controller, control or view subclasses will see corrected values. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (no provider). macOS/Xcode lanes not run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new reads, execution, or network access. ## Author readiness Human owner: @mhamzasakhi - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:43:19 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:45 +00:00

[PR #6827] [MERGED] fix(native): preserve unknown ELF export coverage without a dynsym section #7420

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#6827

No description provided.

Rows
Columns