[Ani07-05]

Closes #264

Changes

• Unwrap scalar map keys/values to their underlying primitive in pub type map aliases (HashMap<SSN, Employee> becomes HashMap<String, Employee>), matching how field sites already behave. Scalars are not emitted, so the alias previously referenced undefined types (bugs 1 and 2)
• Box a field as Box<T> (Option<Box<Manager>>) only when it is recursive, to break the infinite-size type (bug 3). A field is recursive when its target reaches the declaring class through properties (direct or indirect cycle), through a superclass or subclass, or, with flattenSubclassesToUnion, through any variant of the <Base>Union enum
• Remove the Rust skip entries for hr_base and hr_integration in test/verification/cases.js so they are verified with cargo check
• Add unit tests for scalar map unwrapping, direct/indirect cycles, subclass and union recursion, and the cases that stay unboxed; update codegen snapshots

Flags

• Boxing is conditional, so output only changes for recursive fields (see snapshot diff). Non-recursive relationships are emitted as before
• Vec<T> fields and map values are left unboxed since they already provide indirection
• Union variants are limited to direct subclasses declared in the same model file, matching how the union enum is generated

Testing

• npm test: 703 passing, 99% statement coverage; lint, licence-header check and JSDoc clean
• npx mocha test/codegen/fromcto/rust/rustvisitor.js: 62 passing
• Rust verification (cargo check on generated crates): hr_base and hr_integration pass with the skips removed. Five union and recursion cases pass with flattenSubclassesToUnion

Related Issues

• Issue bug: Rust verification - generated output fails cargo check #264

Author Checklist

• Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
• Vital features and changes captured in unit and/or integration tests
• Commits messages follow AP format
• Extend the documentation, if necessary

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[apoorv7g]

@Ani07-05 Has this been well tested ?

[Ani07-05]

Yes, tested. I ran:

• npm test: 686 passing, 0 failing
• npm run lint: eslint, lockfile-lint, syncpack all pass
• cargo check on all 8 Rust verification cases, including hr_base and hr_integration
• Added a unit test for scalar map unwrapping, updated relationship tests and snapshots

[Jason-123-cyber]

ok

[Jason-123-cyber]

jk

[mira-accord-project]

Thank you for this contribution, Ani07-05, and for the clear PR description. Unwrapping scalar map keys and values in visitMapDeclaration is a clean fix for the undefined SSN and Time types, and removing the two Rust skips in test/verification/cases.js is the right follow-through. Boxing non-array relationships does break the Manager cycle in the HR model. Nothing here is blocking, but a few things would make this ready to merge: (1) rebase onto main and regenerate the snapshot, (2) either box recursive containment fields as well or narrow the title and description to relationship recursion, (3) list the unconditional boxing as a numbered question for the maintainers and consider limiting it to relationships that are actually recursive, and (4) add a unit test for a scalar map value.

Nothing blocking; some suggestions below. Findings: 3 should-fix, 1 nit.

• should-fix · hygiene: GitHub reports this branch as conflicting with main. Please rebase, run npm run test:updateSnapshots, and read the changes in test/codegen/__snapshots__/codegen.js.snap to check that each changed line still follows from the fix. The commit also needs to be able to pass npm test on the rebased branch.

---

_{Automated review by Mira, an Accord Project agent, at f7c8295. Findings are labelled blocking, should-fix or nit; a maintainer makes the final call.}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[mira-accord-project]

Thank you for the updates, Ani07-05. The new head addresses the earlier feedback well. Boxing now happens only where a property really reaches its declaring class, which keeps the generated output unchanged for most models. Containment fields are covered as well as relationships. The snapshot change is limited to Employee.manager and Manager.manager. The scalar key and value map handling has unit tests, and the two stale verification skips are removed. One edge case remains, under the opt-in flattenSubclassesToUnion parameter. Once that is handled, with a unit test, this looks ready for a maintainer to approve. The earlier review threads look resolved by these changes, so it would help to mark them resolved. The CI workflows for a first contribution need a maintainer to approve them, so only the DCO check has run so far.

Nothing blocking; some suggestions below. Findings: 1 should-fix.

• should-fix · correctness · lib/codegen/fromcto/rust/rustvisitor.js: isRecursive only boxes when the target is the declaring class, reaches it through properties, or is a subclass of it. With flattenSubclassesToUnion on, a field typed with a superclass is emitted as <Base>Union, and that enum has a variant for every subclass. Take abstract concept Base {} and concept A extends Base { o Base other }. This generates A { other: BaseUnion } and BaseUnion::A(A), which is an infinitely sized type that cargo check rejects with E0072. isRecursive returns false here because Base never reaches A. Suggested fix: pass useUnion into isRecursive. When it is true, also treat the property as recursive if the owner is, or inherits from, the property's target type (compare owner.getAllSuperTypeDeclarations() with the target) and the target has subclasses in the same model file. Please add a unit test for this case alongside the other recursion tests.

---

_{Automated review by Mira, an Accord Project agent, at ae6fb34. Findings are labelled blocking, should-fix or nit; a maintainer makes the final call.}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[mira-accord-project]

Thank you for sticking with this, Ani07-05. The latest commit is a clear improvement. Boxing now happens only when a property reaches its declaring class, which covers direct, indirect, subclass and flattenSubclassesToUnion cases and leaves other output unchanged. Earlier comments about containment recursion, unconditional boxing and missing scalar-value tests are addressed, and the new unit tests for the cycle and union cases are well chosen. I found no new problems in the code. To make this mergeable: (1) the test and verification workflows haven't run on this commit, so cargo check for the Rust cases whose skips you removed is not yet confirmed. A maintainer needs to approve the workflow runs, and they need to pass; (2) please resolve the three earlier review threads that are now addressed; (3) a maintainer needs to approve. Please also include the commands you ran locally and the pass counts in the description, as the repo's guidance asks.

No problems found.

---

_{Automated review by Mira, an Accord Project agent, at 6938e9c. Findings are labelled blocking, should-fix or nit; a maintainer makes the final call.}

[Ani07-05]

@mttrbrts could you approve the CI workflow runs ?

[Copilot]

🟡 Changes recommended

Nested fields bypass emitted unions during cycle detection, leaving some multi-union cycles unboxed and causing Rust compilation to fail with infinite-size types.

1 open issue

• Flattened subclass unions can leave recursive types unboxed · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Nested union variants are not expanded during recursion checks, leaving mutual by-value cycles unboxed and causing generated Rust to fail compilation with infinite-size type errors.

1 open issue

• Flattened subclass unions can leave recursive types unboxed

_{Review effort: Balanced}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.