Skip to content

Split morphir-extension-sdk's native.rs into focused modules #196

Description

@DamianReeves

What happens

crates/morphir-extension-sdk/src/native.rs carries several distinct concerns in one file: the typed native endpoint traits, the role records and their projections, the consuming builder and its typestate, the protocol handle, the doctest fixtures, and a large test module.

AGENTS.md:88-101 asks for a split well before this size:

Module-per-Directory Pattern: When a module grows beyond ~300-500 lines or contains multiple related types
Complex implementations: Consider splitting at 500+ lines
If a file has multiple impl blocks for different concerns, split by concern

The file was already over the guideline before recent work (885 lines); the native role-registration refactor in #195 grew it to 1,325, and the native workspace role in #198 grew it further. Codex raised it on #195 as a P1 and it is correct.

Ready to pick up when all four hold

Run each check from the repository root. If any is false, stop — the boundaries are still moving and the split will have to be re-cut.

# Condition Check Expected when ready
1 The role-registration refactor is merged gh pr view 195 --repo finos/morphir-rust --json state --jq .state MERGED ✅
2 Guest registration is consolidated rg '__push_extension_types|__push_extension_dispatchers' crates/morphir-extension-sdk/src/lib.rs no matches (today: 15)
3 The native workspace role exists rg 'pub fn with_workspace' crates/morphir-extension-sdk/src/native.rs at least one match ✅ (landed in #198)
4 The stronger authoring descriptors have landed rg 'capability: FrontendCapability|capability: BackendCapability' crates/morphir-extension-sdk/src/native.rs no matches in the role-record definitions (today: 7)

Correction to an earlier version of this issue. Condition 3 previously checked that rg 'advertises workspace without a native workspace handle' returned no matches, on the assumption that adding the role would delete that message. It does not, and should not: #198 kept the message verbatim, moved into the (false, Some(_)) arm of validate_capabilities, where it still correctly describes an extension that advertises a workspace capability without registering a handle. That check could therefore never reach zero, and would have reported this precondition as unmet forever. The signal above — the presence of the builder method that registers the role — actually distinguishes the two states.

Why each one

(1) is obvious but worth stating: this issue is about the file #195 produced. Satisfied.

(2) The guest consolidation folds export_extension! onto one registration declaration, retiring the separate __push_extension_types! / __push_extension_dispatchers! authoring paths. It mostly touches lib.rs, so it is the weakest of the four preconditions for this file — but it is the change most likely to move code across the lib.rs / native.rs boundary, which is exactly the boundary a split has to draw.

(3) The native workspace role added a third role: a NativeWorkspace endpoint trait, a WorkspaceRole record, a workspace slot on NativeRoles, a builder method, and the replacement of an unconditional rejection with ordinary presence reconciliation. This was the single largest structural change pending for native.rs. Satisfied by #198.

(4) The stronger authoring descriptors replace the stored FrontendCapability / BackendCapability in each role record with a descriptor that has no configurable compile / generate flag, with the projection supplying true. That changes what a role record is, which is the thing a role-records module would be organised around.

Conditions (2) and (4) both flip when the authoring-API PR lands — the guest consolidation and the stronger descriptors were regrouped into one PR, since they are the same subsystem: what an author declares. Picking this up early is not blocked in any hard sense — it is just work that will be partly redone.

What done looks like

  • A native/ directory with mod.rs exposing the public API, implementation split by concern — roughly: endpoint traits, role records and projections, builder and typestate, protocol handle, doctest fixtures. Tests move alongside the concern they cover.
  • No file over ~400 lines. If a concern will not fit, that is a signal about the concern, not a reason to raise the number.
  • The crate-root re-export is unchanged: pub use native::{NativeBackend, NativeExtension, NativeFrontend, NativeProtocol, NativeWorkspace};, and the prelude's re-export of the same five stays intact. prelude_exports_every_native_adapter will catch a dropped one.
  • No consumer changes. morphir-daemon and the parent finos/morphir must build untouched. If either needs an edit, the public API moved and the split overstepped.
  • Behaviour-preserving: no test modified except for module paths. A test whose assertions change means something other than a file move happened.

Decide at the same time

Empty and NonEmpty are public names in native that claim more generality than "the builder has no roles yet", and read poorly beside the WorkspaceRole / future TransformRole vocabulary. A builder submodule is their natural home. Renaming is cheap while they have few callers and gets steadily less so.

Why it was not done in #195

Deliberately deferred:

  • refactor(sdk): one authoritative role registration for native extensions #195's binding constraint was that nothing observable changes. A large mechanical file move on top of a behaviour-preservation refactor obscures the property under review — confirming construction order, error precedence and wire shape were untouched is materially harder to see through a reorganisation.
  • The design spec for this work lists combining structural reorganisation with the refactor as an explicit trap.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions