morluto/rea

Fork 0

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

[PR #4085] [MERGED] fix(domain): keep a decoded Swift alias category beside undecoded aliases #6048

New issue

Closed

opened 2026-10-31 01:11:27 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:27 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4085
Author: @kaoru0822-kitauji
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/3934-swift-alias-category

📝 Commits (1)

07ff18d fix(domain): keep a decoded Swift alias category beside undecoded aliases

📊 Changes

2 files changed (+38 additions, -10 deletions)

View changed files

📝 src/domain/symbolAnalysis.test.ts (+30 -0)
📝 src/domain/symbolAnalysis.ts (+8 -10)

📄 Description

Summary

Fixes #3934. analyze_swift_types let an undecoded alias erase a class category that another alias at the same address had decoded. An alias counts as undecoded when REA recognizes it as Swift but cannot decode its category. In that case the procedure moved to other, and category: "classes" returned total: 0. Only decoded categories now vote, as the code comment already promised. The undecoded alias is kept as evidence in mangled_names.

Problem and expected behavior

categorizeSwiftTypes (src/domain/symbolAnalysis.ts) filtered aliases with classifySwiftSymbol(name) !== undefined. A recognized-but-undecoded alias returns { category: null }, so it passed that filter and added null to the vote set. One decoded category plus null makes two distinct values, so the category was cleared and the entry reported category_not_decoded.

I reproduced this through the production MCP server with the issue's own script (createServer with an in-memory transport; procedure main::Service::init at 0x1):

aliases	filter	main	this PR
decoded only	none / classes	total 1, classes 1, unclassified []	unchanged
decoded + undecoded	none	total 1, classes 0, other 1, `category_not_decoded`	total 1, classes 1, other 0, unclassified []
decoded + undecoded	classes	total 0, `category_not_decoded`	total 1, classes 1, unclassified []
undecoded only	none / classes	other 1 / total 0, `category_not_decoded`	unchanged

The class entry keeps both mangled_names (_$s4main7ServiceCMa, _$s4main04PairVMa).

Change and scope

Only non-null decoded categories are collected as votes. The category is set when exactly one is decoded.
conflicting_categories now means two or more decoded categories disagree. Previously that check had to exclude null explicitly. Every other case is category_not_decoded.
The output schema is unchanged.

Contract and boundary impact

Semantic owner and earliest changed stage: the categorizeSwiftTypes domain categorization, which consumes classifySwiftSymbol's parsed category.
Contribution scope fit and linked maintainer decision: a bug fix within existing behavior; not applicable.
CLI and MCP/tool-catalog contract: schema unchanged. A decoded category is no longer lost when an undecoded alias appears beside it.
Provider, bridge, target-format, or platform compatibility: provider-independent.
Evidence, artifact, provenance, or reconstruction contract: undecoded aliases remain in mangled_names. Entries without any decoded category still appear in unclassified.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: src/domain/symbolAnalysis.test.ts adds the issue's two-alias case, with and without the classes filter. It asserts total: 1, classes.count: 1, other.count: 0, empty unclassified, and both manglings retained. Both cases fail on main (2 failed / 14 passed) and pass with the fix.
Base reproduction: the MCP table above was executed with main's and this branch's built runtime.
Output size and time (issue's 6-call MCP script, cold process): main 4,764 bytes in 489 ms; this PR 4,288 bytes in 486 ms. The size drops because the two misreported unclassified entries are gone. Time is unchanged.
Remaining proof gaps: no run against a real Swift binary through a disassembler provider. Ghidra and Hopper sessions were not available on this host, so the provider inventory was simulated, as in the issue's script.

Validation performed

npx vitest run src/domain/symbolAnalysis.test.ts — 16 passed; against main, 2 failed as expected.
npx vitest run src/domain tests/boundary/mcp/enhancedTools.swiftTypes.test.ts tests/boundary/mcp/enhancedTools.inputValidation.test.ts tests/boundary/mcp/enhancedTools.03-returns-typed-graph-failures-and.test.ts tests/boundary/mcp/enhancedTools.01-lists-the-complete-target-open.test.ts tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts — 187 files, 3,219 passed.
npm run check:fast — passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: none; the provider inventory was simulated (see gaps above).
Package or release metadata impact: none.
Security, privacy, process, or containment review: pure categorization logic.

Author readiness

Human owner:

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.

🤖 Generated with Claude Code

_{🔄 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/4085 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/3934-swift-alias-category` --- ### 📝 Commits (1) - [`07ff18d`](https://github.com/morluto/rea/commit/07ff18dbd58256dde368227830f27769ca48b957) fix(domain): keep a decoded Swift alias category beside undecoded aliases ### 📊 Changes **2 files changed** (+38 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/symbolAnalysis.test.ts` (+30 -0) 📝 `src/domain/symbolAnalysis.ts` (+8 -10) </details> ### 📄 Description ## Summary Fixes #3934. `analyze_swift_types` let an undecoded alias erase a class category that another alias at the same address had decoded. An alias counts as undecoded when REA recognizes it as Swift but cannot decode its category. In that case the procedure moved to `other`, and `category: "classes"` returned `total: 0`. Only decoded categories now vote, as the code comment already promised. The undecoded alias is kept as evidence in `mangled_names`. ## Problem and expected behavior `categorizeSwiftTypes` (`src/domain/symbolAnalysis.ts`) filtered aliases with `classifySwiftSymbol(name) !== undefined`. A recognized-but-undecoded alias returns `{ category: null }`, so it passed that filter and added `null` to the vote set. One decoded category plus `null` makes two distinct values, so the category was cleared and the entry reported `category_not_decoded`. I reproduced this through the production MCP server with the issue's own script (`createServer` with an in-memory transport; procedure `main::Service::init` at `0x1`): | aliases | filter | main | this PR | |---|---|---|---| | decoded only | none / classes | total 1, classes 1, unclassified [] | unchanged | | decoded + undecoded | none | total 1, **classes 0, other 1**, `category_not_decoded` | total 1, **classes 1, other 0**, unclassified [] | | decoded + undecoded | classes | **total 0**, `category_not_decoded` | **total 1, classes 1**, unclassified [] | | undecoded only | none / classes | other 1 / total 0, `category_not_decoded` | unchanged | The class entry keeps both `mangled_names` (`_$s4main7ServiceCMa`, `_$s4main04PairVMa`). ## Change and scope - Only non-null decoded categories are collected as votes. The category is set when exactly one is decoded. - `conflicting_categories` now means two or more *decoded* categories disagree. Previously that check had to exclude `null` explicitly. Every other case is `category_not_decoded`. - The output schema is unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: the `categorizeSwiftTypes` domain categorization, which consumes `classifySwiftSymbol`'s parsed category. - Contribution scope fit and linked maintainer decision: a bug fix within existing behavior; not applicable. - CLI and MCP/tool-catalog contract: schema unchanged. A decoded category is no longer lost when an undecoded alias appears beside it. - Provider, bridge, target-format, or platform compatibility: provider-independent. - Evidence, artifact, provenance, or reconstruction contract: undecoded aliases remain in `mangled_names`. Entries without any decoded category still appear in `unclassified`. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `src/domain/symbolAnalysis.test.ts` adds the issue's two-alias case, with and without the `classes` filter. It asserts `total: 1`, `classes.count: 1`, `other.count: 0`, empty `unclassified`, and both manglings retained. Both cases fail on main (2 failed / 14 passed) and pass with the fix. - Base reproduction: the MCP table above was executed with main's and this branch's built runtime. - Output size and time (issue's 6-call MCP script, cold process): main 4,764 bytes in 489 ms; this PR 4,288 bytes in 486 ms. The size drops because the two misreported `unclassified` entries are gone. Time is unchanged. - Remaining proof gaps: no run against a real Swift binary through a disassembler provider. Ghidra and Hopper sessions were not available on this host, so the provider inventory was simulated, as in the issue's script. ## Validation performed - `npx vitest run src/domain/symbolAnalysis.test.ts` — 16 passed; against main, 2 failed as expected. - `npx vitest run src/domain tests/boundary/mcp/enhancedTools.swiftTypes.test.ts tests/boundary/mcp/enhancedTools.inputValidation.test.ts tests/boundary/mcp/enhancedTools.03-returns-typed-graph-failures-and.test.ts tests/boundary/mcp/enhancedTools.01-lists-the-complete-target-open.test.ts tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts` — 187 files, 3,219 passed. - `npm run check:fast` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none; the provider inventory was simulated (see gaps above). - Package or release metadata impact: none. - Security, privacy, process, or containment review: pure categorization logic. ## Author readiness Human owner:  - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:11:27 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6143] [MERGED] fix(cancellation): preserve caller abort reasons through late finalization #7158

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#6048

No description provided.

Rows
Columns