Skip to content

refactor: slim DeviceSpec to required fields + sparse serialization#3170

Merged
steviec merged 18 commits intomainfrom
slimmer-device-spec
Apr 15, 2026
Merged

refactor: slim DeviceSpec to required fields + sparse serialization#3170
steviec merged 18 commits intomainfrom
slimmer-device-spec

Conversation

@steviec
Copy link
Copy Markdown
Collaborator

@steviec steviec commented Apr 11, 2026

Stack: base main. Coordinated rollout: mobile-dev-inc/copilot#2398 (worker, deploy first) → this PR → mobile-dev-inc/copilot#2399 (backend, deploy last).

Problem

DeviceSpec required callers to provide values that have permanent, never-changing defaults (locale, cpuArchitecture, etc.) and persisted a handful of fields that are pure derivations (osVersion, deviceName, tag, emulatorImage). The result: verbose, noisy payloads, lots of unnecessary stored values in the DB, and confusion about which fields actually carry intent.

It also mixed two different concerns: provisioning identity (what device to boot) and driver-level runtime config (how to configure the driver after boot). Driver-level config already lives in WorkspaceConfig, where the CLI has always read it from; only the cloud worker was pulling it from DeviceSpec.

Separately, DeviceSpecRequest was a parallel nullable-everything hierarchy that existed only so fromRequest() could magically fill in defaults — indirection without a clear reason to exist now that Kotlin constructor defaults do the same thing.

Approach

  • DeviceSpec becomes provisioning identity only: platform, model, os, locale, plus cpuArchitecture on Android.
  • Driver-level fields (disableAnimations, snapshotKeyHonorModalViews, orientation) are removed — consumers read them from WorkspaceConfig.platform instead.
  • Require only platform + model + os from callers. Everything else has a sensible default in the primary constructor.
  • Derived values (osVersion, deviceName, tag, emulatorImage) become computed get() properties — not stored, not serialized.
  • Sparse JSON serialization: the wire format contains only the platform discriminator, the required fields, and any optional field whose runtime value differs from the constructor default. Single source of truth for defaults = the constructor signature, read via Kotlin reflection.
  • Named DEFAULT constant per subtype (DeviceSpec.Android.DEFAULT, etc.) for call sites that just want a reasonable device.

Final shape

data class Android(
    override val model: String,
    override val os: String,
    val locale: AndroidLocale = AndroidLocale.fromString("en_US"),
    val cpuArchitecture: CPU_ARCHITECTURE = CPU_ARCHITECTURE.ARM64,
) : DeviceSpec()

data class Ios(
    override val model: String,
    override val os: String,
    val locale: IosLocale = IosLocale.EN_US,
) : DeviceSpec()

data class Web(
    override val model: String,
    override val os: String,
    val locale: WebLocale = WebLocale.EN_US,
) : DeviceSpec()

Key changes

  • DeviceSpec.kt: Reshaped to provisioning-only. DeviceSpecRequest and DeviceSpec.fromRequest() deleted. Driver fields removed.
  • DeviceSpecSparseSerializer.kt (new): custom JsonSerializer<DeviceSpec> that reads primary constructor defaults via reflection (cached per subtype) and omits fields matching defaults.
  • CLI call sites updated to use DeviceSpec.X.DEFAULT instead of scattered hardcoded model/os strings.

Wire format

Android with only required fields:

{"platform":"ANDROID","model":"pixel_6","os":"android-33"}

With a non-default cpuArchitecture:

{"platform":"ANDROID","model":"pixel_6","os":"android-33","cpuArchitecture":"X86_64"}

Coordination with copilot

This PR deliberately removes driver fields that the copilot worker currently reads. The rollout is phased:

  1. copilot#2398 (worker) — worker reads disableAnimations/snapshotKeyHonorModalViews from WorkspaceConfig. Deploy first.
  2. This PR — merge after worker deploys so no running worker still reads the removed fields.
  3. copilot# (backend) — bump submodule to this PR's head, simplify backend ingestion. Deploy last.

Jackson FAIL_ON_UNKNOWN_PROPERTIES = false on the backend and worker mappers ensures existing DB rows with the now-removed fields continue to deserialize cleanly.

Test plan

  • Full compile clean across all modules
  • Full test suite passes
  • Sparse serializer tests cover locale + cpuArchitecture default-omission
  • Legacy-verbose-JSON deserialization tests cover the full set of now-removed fields
  • Manual smoke: `maestro start-device --platform android` launches a default emulator
  • Manual smoke: `maestro start-device --platform ios --device-model iPhone-14 --device-os iOS-17-5` launches a simulator

Copy link
Copy Markdown
Contributor

@proksh proksh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice Simplication @steviec 🙌

Comment thread maestro-client/src/main/java/maestro/device/DeviceSpec.kt
@steviec steviec force-pushed the slimmer-device-spec branch from f477763 to 9a7aaf7 Compare April 14, 2026 17:46
@pedro18x
Copy link
Copy Markdown
Contributor

Does the CLI still populate deviceLocale/deviceModel/deviceOs alongside deviceSpec in the upload payload? Diff only shows the import change, so I can't tell from here. If it does, every post-PR3 CLI run 400s on MixedDeviceSpecRequestException. Worth a test pinning "deviceSpec set, flat fields null."

Copy link
Copy Markdown
Contributor

@pedro18x pedro18x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sparse serializer approach is awesome. One coordination question above that I'd want nailed down before this and 2399 ship together.

@steviec
Copy link
Copy Markdown
Collaborator Author

steviec commented Apr 14, 2026

Does the CLI still populate deviceLocale/deviceModel/deviceOs alongside deviceSpec in the upload payload? Diff only shows the import change, so I can't tell from here. If it does, every post-PR3 CLI run 400s on MixedDeviceSpecRequestException. Worth a test pinning "deviceSpec set, flat fields null."

@pedro18x no, the CLI doesn't use the deviceSpec payload to the API yet.

@pedro18x pedro18x self-requested a review April 14, 2026 22:52
@pedro18x
Copy link
Copy Markdown
Contributor

Does the CLI still populate deviceLocale/deviceModel/deviceOs alongside deviceSpec in the upload payload? Diff only shows the import change, so I can't tell from here. If it does, every post-PR3 CLI run 400s on MixedDeviceSpecRequestException. Worth a test pinning "deviceSpec set, flat fields null."

@pedro18x no, the CLI doesn't use the deviceSpec payload to the API yet.

I see. Approved!

steviec added 18 commits April 15, 2026 10:13
…lts, and computed properties

Required: model, os. Optional with defaults: locale (narrowed per subtype),
orientation, disableAnimations, cpuArchitecture, snapshotKeyHonorModalViews.
Derived values (osVersion, deviceName, tag, emulatorImage) are now computed
get() properties rather than stored fields. Deletes DeviceSpecRequest and
DeviceSpec.fromRequest(); callers will be updated in follow-up commits.
Avoids running fromString() validation on every default construction.
AndroidLocale is a data class (not an enum) so its default remains
AndroidLocale.fromString("en_US") for now.
…irectly

Removes DeviceSpecRequest usage and explicitly passes model/os defaults at
each construction site. Uses ifBlank fallbacks for cases where AvdInfo or
simctl parsing may produce empty strings.
…pec directly

Threads CLI-provided device options into platform-specific DeviceSpec
constructors with explicit fallbacks for missing model/os/locale values.
Remove stale DeviceSpecRequest import from CloudInteractor and replace
the DeviceSpec.fromRequest(DeviceSpecRequest.Android()) demo call in
AnsiResultView.main() with a direct DeviceSpec.Android(model, os) construction.
Smart-cast deviceSpec to DeviceSpec.Ios in AppValidator.validateDeviceCompatibility
before accessing osVersion, which is now a subtype-local property.
…ializers

Custom JsonSerializer<DeviceSpec> reads primary constructor defaults via
Kotlin reflection and omits fields whose runtime value matches the default.
Produces wire format containing only platform, model, os, and any fields
that deviate from defaults.

Adds per-subtype locale deserializers (AndroidLocale, IosLocale, WebLocale)
because the narrowed locale field types on DeviceSpec subclasses cannot
accept the base DeviceLocale interface returned by the legacy deserializer.
The sparse serializer iterates only primary-constructor parameters, so
computed get() properties are already excluded regardless of annotation.
Removes serialization coupling from the data class.
Replaces scattered hardcoded model/os strings at call sites with a single
named DEFAULT constant per subtype. Parse-or-default sites branch to the
full default when input is incomplete.
Removes disableAnimations, snapshotKeyHonorModalViews, and orientation
from DeviceSpec subtypes. These are driver-level concerns sourced from
WorkspaceConfig, not provisioning identity. Copilot-side worker already
reads these values from WorkspaceConfig (see copilot#2398).
…sealed class

Lets callers access these on a base DeviceSpec without per-subtype casts.
Required by the copilot backend's response builders that work with a
DeviceSpec without knowing its concrete subtype.
@steviec steviec force-pushed the slimmer-device-spec branch from 9a7aaf7 to b9c537f Compare April 15, 2026 17:14
@steviec steviec merged commit b4c88bf into main Apr 15, 2026
9 checks passed
@steviec steviec deleted the slimmer-device-spec branch April 15, 2026 17:46
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.

4 participants