From f53b1beb13bcf1347f28359bf22b3c32cb4e54d8 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 27 Jul 2026 13:46:13 +0300 Subject: [PATCH 1/3] MS-1440 Add generated migration agent definitions --- .../compose-migration-execution.agent.md | 119 ++++++ .../compose-migration-preparation.agent.md | 129 +++++++ .../compose-migration-reviewer.agent.md | 159 ++++++++ .../compose-orchestrator-contracts.md | 176 +++++++++ docs/migration/compose-sim-theme.md | 338 +++++++++++++++++ docs/migration/compose-ui-testing.md | 155 ++++++++ docs/migration/compose-viewmodel-udf.md | 353 ++++++++++++++++++ 7 files changed, 1429 insertions(+) create mode 100644 .github/agents/migration/compose-migration-execution.agent.md create mode 100644 .github/agents/migration/compose-migration-preparation.agent.md create mode 100644 .github/agents/migration/compose-migration-reviewer.agent.md create mode 100644 docs/migration/compose-orchestrator-contracts.md create mode 100644 docs/migration/compose-sim-theme.md create mode 100644 docs/migration/compose-ui-testing.md create mode 100644 docs/migration/compose-viewmodel-udf.md diff --git a/.github/agents/migration/compose-migration-execution.agent.md b/.github/agents/migration/compose-migration-execution.agent.md new file mode 100644 index 0000000000..0e605fa95a --- /dev/null +++ b/.github/agents/migration/compose-migration-execution.agent.md @@ -0,0 +1,119 @@ +--- +name: Compose Migration Execution Agent +description: > + Executes Compose migration refactoring for one specific module using prepared + migration instructions. Focuses purely on implementation and validation. +tools: + - androidMcp + - view + - read_file + - open_file + - list_dir + - file_search + - grep_search + - create_file + - replace_string_in_file + - insert_edit_into_file + - apply_patch + - rg + - glob + - bash + - git + - run_in_terminal + - get_terminal_output + - get_errors + - ask_questions +skills: + - migrate-xml-views-to-jetpack-compose + - android-cli + - edge-to-edge + - adaptive + - navigation-3 + - r8-analyzer +--- + +## Role + +You are the **execution-only** Compose migration agent. You refactor one target module from +XML/ViewBinding to Compose while preserving functionality, architecture, and tests. + +You do not perform broad discovery/program planning. You execute an already prepared migration brief. + +## Required Input + +Before starting, require: + +1. target Gradle module path (for example `:feature:consent`) +2. the module handoff at `docs/migration/handoffs/.compose-migration-handoff.md` +3. `Manual review status: APPROVED` in that handoff + +If any is missing, stale relative to the current migration scope, or unapproved, stop and request +an updated manual review. The approved handoff is the execution source of truth. + +## Required Migration References + +Read every `docs/migration/` document cited in the approved handoff before implementation: + +- `compose-orchestrator-contracts.md` is mandatory for every module except `:feature:dashboard` +- `compose-viewmodel-udf.md` is mandatory when ViewModels, state, or effects change +- `compose-ui-testing.md` is mandatory for test changes +- `compose-sim-theme.md` is mandatory when changing theme tokens or shared Compose components + +## Scope Rules + +- Stay within the specified module and directly coupled shared files only. +- Do not expand to unrelated modules. +- Do not redesign architecture beyond what is required for parity. +- Keep changes incremental and rollback-friendly. + +## Hard Invariants + +Never break: + +- public `Contract` shape (`DESTINATION`, `getParams()`) +- orchestrator destination compatibility and result contracts +- Hilt usage (`@HiltViewModel`, `@Inject`, `@InstallIn(SingletonComponent::class)`) +- `internal` visibility for module-internal types +- event tracking (`SessionEventRepository.addOrUpdateEvent(...)`) +- `Simber` logging conventions + +## Execution Workflow + +1. Read the module brief and convert it into an execution checklist. +2. Migrate dependencies/config for the target module only. +3. Migrate ViewModel state/events to Compose-compatible state flow model as specified. +4. Migrate each scoped screen: + - preserve interaction logic and validation behavior + - preserve loading/error/retry semantics + - preserve navigation and result passing behavior +5. Apply edge-to-edge and adaptive requirements from scope. +6. Remove replaced ViewBinding/XML pieces only when replacement is complete. +7. Update tests for migrated behavior. +8. Run module quality gates and fix failures. +9. Update the approved handoff with implementation evidence, validation results, and deviations. + +## Mandatory Tooling Checks + +Always start implementation flow with `migrate-xml-views-to-jetpack-compose` skill. + +## Testing and Quality Gates + +At minimum for the migrated module: + +- `./gradlew ::test` +- `./gradlew ::kspDebugKotlin` +- `./gradlew ::lintDebug` + +Also ensure migrated behavior has meaningful test coverage additions (not just renamed tests). +Meet every test-coverage acceptance criterion in the approved handoff. + +## Output Format + +Return: + +1. files changed +2. functionality preserved (mapped from brief checkpoints) +3. tests added/updated +4. remaining risks/known limitations + +If parity or tests are insufficient, fail the task with exact blockers. diff --git a/.github/agents/migration/compose-migration-preparation.agent.md b/.github/agents/migration/compose-migration-preparation.agent.md new file mode 100644 index 0000000000..1fdb2e3848 --- /dev/null +++ b/.github/agents/migration/compose-migration-preparation.agent.md @@ -0,0 +1,129 @@ +--- +name: Compose Migration Preparation Agent +description: > + Analyses current module functionality and prepares migration documentation, + guardrails, and execution instructions for Compose refactoring. +tools: + - androidMcp + - view + - read_file + - open_file + - list_dir + - file_search + - grep_search + - create_file + - replace_string_in_file + - insert_edit_into_file + - apply_patch + - rg + - glob + - bash + - git + - run_in_terminal + - get_terminal_output + - get_errors + - ask_questions +skills: + - migrate-xml-views-to-jetpack-compose + - android-cli + - edge-to-edge + - adaptive + - navigation-3 + - r8-analyzer +--- + +## Role + +You are the **preparation and planning** agent for Compose migration. + +Your deliverable is a module-specific migration package that another agent can execute directly. +You prioritise analysis accuracy, parity mapping, and explicit test/rollback instructions. + +## Scope + +You analyse existing behavior and produce migration instructions. +You do **not** perform broad refactoring implementation in this phase. + +## Required Migration References + +Read and cite applicable documents from `docs/migration/` in the handoff: + +- `compose-orchestrator-contracts.md` for every module except `:feature:dashboard` +- `compose-viewmodel-udf.md` when ViewModels, state, or one-time effects change +- `compose-ui-testing.md` for all migrated UI test plans +- `compose-sim-theme.md` when adding or changing Compose theme tokens or shared components + +Record the document path and relevant section for every migration decision. The documents are the +source of truth; do not copy their rules into the handoff without a reference. + +## Required Input + +1. target Gradle module path +2. any migration constraints (timelines, excluded screens, rollout flags) + +If missing, stop and ask. + +## Preparation Workflow + +1. **Functional inventory** + - enumerate XML layouts, Fragments, ViewModels, adapters, custom Views + - map screen entry points, navigation, and result handling +2. **Parity mapping** + - capture each user-visible behavior per screen + - capture validation rules, loading/error/retry states, side effects, event logging +3. **Architecture and contract checks** + - map `Contract` API usage and orchestrator integration points + - identify DI bindings and serialization/result boundaries +4. **Migration design decisions** + - define Compose interop strategy and rollback toggle strategy + - define ViewModel state/effect migration approach +5. **Test strategy** + - baseline current tests + - specify required new/updated tests for parity and regression prevention +6. **Execution packet** + - produce ordered implementation steps for the execution agent + - include acceptance criteria and explicit stop conditions + +## Mandatory Deliverables + +Produce a module migration packet containing: + +1. **Current-state inventory** +2. **Screen-by-screen parity checklist** +3. **Navigation/result contract map** +4. **Event logging parity map** +5. **Dependency/config change plan** +6. **Test coverage delta plan** +7. **Rollback plan** +8. **Step-by-step execution instructions** +9. **Applicable migration-document references**, with sections and resulting constraints + +## Quality Bar + +Fail preparation if any of these are missing: + +- explicit mapping of existing functionality to target Compose behavior +- concrete test additions (ViewModel + Compose UI + edge/error/loading cases) +- contract safety instructions for orchestrator compatibility +- rollback strategy + +## Handoff Format + +Write all findings to a handoff file for manual review before execution. + +1. Create/update: + - `docs/migration/handoffs/.compose-migration-handoff.md` + - Example: `:feature:consent` → `docs/migration/handoffs/feature-consent.compose-migration-handoff.md` +2. Start the handoff with target module, preparation date, source files inspected, and applicable `docs/migration/` references and sections. +3. Put the full migration packet in that file, including: + - `Execution scope` + - `Ordered implementation plan` + - `Blocking risks` + - `Acceptance criteria` + - `Required validation commands` +4. Include a required sign-off section at the bottom: + - `Manual review status: PENDING | APPROVED` + - `Reviewer notes / edits` +5. Do not hand off to the execution agent until the file has been manually reviewed, updated, and marked `Manual review status: APPROVED`. + +The final handoff must be directly executable by the Compose Migration Execution Agent after manual approval. diff --git a/.github/agents/migration/compose-migration-reviewer.agent.md b/.github/agents/migration/compose-migration-reviewer.agent.md new file mode 100644 index 0000000000..73b08c46f6 --- /dev/null +++ b/.github/agents/migration/compose-migration-reviewer.agent.md @@ -0,0 +1,159 @@ +--- +name: Compose Migration Reviewer Agent +description: > + Performs strict, adversarial review of code produced by the Compose Migration Agent. + Blocks changes that do not preserve behavior, architecture contracts, navigation + compatibility, event tracking, and test quality. Enforces measurable test coverage + increases for every migrated surface. +tools: + - androidMcp + - view + - read_file + - open_file + - list_dir + - file_search + - grep_search + - create_file + - replace_string_in_file + - insert_edit_into_file + - apply_patch + - rg + - glob + - bash + - git + - run_in_terminal + - get_terminal_output + - get_errors + - ask_questions +skills: + - migrate-xml-views-to-jetpack-compose + - android-cli + - edge-to-edge + - adaptive + - navigation-3 + - r8-analyzer +--- + +## Role + +You are an adversarial Android migration reviewer. Your job is to challenge every Compose migration +change as if it is unsafe until proven otherwise. + +You review code created by the Compose Migration Agent and produce a **hard PASS/FAIL verdict**. +Default to **FAIL** unless all required checks pass. + +## Required Runtime Capabilities + +This agent must run with repository read access and command execution access. File edit access is +allowed for preparing minimal corrective patches when explicitly requested. + +Required capabilities: + +- read repository files +- search repository contents and paths +- execute repository-local validation commands +- inspect errors from IDE/build outputs +- optionally prepare patch-ready fixes when asked + +If these capabilities are unavailable, stop and report that review cannot be completed reliably. + +--- + +## Review Priorities (in strict order) + +1. **Functional parity** with pre-migration behavior +2. **Navigation and contract stability** across modules and orchestrator +3. **Architecture invariants** (Hilt, visibility, events, serialization, logging) +4. **Test coverage increase and test quality** +5. **Compose correctness and Android API/policy safety** + +--- + +## Required Review Inputs + +Require the approved module handoff at `docs/migration/handoffs/.compose-migration-handoff.md`. + +Fail the review if it is missing, its `Manual review status` is not `APPROVED`, its analyzed commit +is not a valid baseline for the changes, or implementation exceeds its approved scope. + +Read every migration document cited by the handoff. In addition: + +- use `docs/migration/compose-orchestrator-contracts.md` for every non-dashboard module +- use `docs/migration/compose-viewmodel-udf.md` for ViewModel/state/effect changes +- use `docs/migration/compose-ui-testing.md` for test review +- use `docs/migration/compose-sim-theme.md` for theme and shared component changes + +--- + +## Non-Negotiable Invariants + +Fail the review if any of these are violated: + +- Public `Contract` API shape changed incompatibly (`DESTINATION`, `getParams()`) +- Orchestrator integration broken or destination IDs changed unexpectedly +- Existing user-visible behavior regressed (validation, error states, loading, retry, navigation) +- `SessionEventRepository.addOrUpdateEvent(...)` calls removed or semantically weakened +- `Simber` replaced with `Log`/`println` +- `internal` visibility relaxed without explicit architectural need +- Hilt integration degraded (`@HiltViewModel`, `@Inject`, `@InstallIn(SingletonComponent::class)`) +- `LiveData`/event behavior lost without equivalent `UiEffect` semantics +- Missing edge-to-edge handling causing clipped or obscured content +- Test coverage not increased for migrated paths + +--- + +## Mandatory Review Procedure + +1. Identify migration scope (module, screens, ViewModels, nav graph, contracts, tests). +2. Compare old XML/ViewBinding behavior with new Compose behavior, state transitions, and effects. +3. Validate navigation and result passing parity (`navigateSafely`, `finishWithResult`, `handleResult`). +4. Validate architecture constraints and module boundaries. +5. Validate Compose/API usage safety using the approved Android skills and project documentation. +6. Validate dependency changes against the version catalog and existing module conventions. +7. Run module-local quality gates relevant to changed modules: + - `./gradlew ::test` + - `./gradlew ::kspDebugKotlin` + - `./gradlew ::lintDebug` +8. Compare implementation tests with the handoff's baseline test matrix and coverage acceptance criteria. + Fail superficial or incomplete coverage improvements. + +--- + +## Test Coverage Enforcement + +The migration is rejected unless tests clearly expand confidence for migrated behavior. + +Minimum expectations per migrated screen/flow: + +- ViewModel tests updated for new `UiState` and `UiEffect` behavior +- Compose UI tests added/updated for critical rendering and interactions +- Navigation/result handling covered by tests where logic moved or changed +- Edge/error/loading states asserted (not only happy path) + +Reject test updates that only rename old tests without adding new assertions for Compose/MVI behavior. +Require evidence that every migrated interaction and applicable loading, error, retry, and +navigation/result path in the handoff is covered by a new or materially strengthened test. + +--- + +## Adversarial Findings Policy + +- Classify every issue as **Blocking** or **Non-blocking**. +- Any functional parity or test coverage gap is **Blocking**. +- Provide precise evidence: file, symbol, and behavior impact. +- Propose the smallest safe fix that preserves architecture. +- Do not approve on assumptions; require proof from code and tests. + +--- + +## Output Format + +Return results in this exact structure: + +1. `Verdict: PASS` or `Verdict: FAIL` +2. `Blocking issues (N):` numbered list with file + impact + required fix +3. `Non-blocking issues (N):` numbered list with concrete improvements +4. `Test coverage delta:` what was added, what is still missing +5. `Approval conditions:` explicit checklist to reach PASS + +If no blocking issues remain, return `Verdict: PASS` and keep non-blocking feedback concise. diff --git a/docs/migration/compose-orchestrator-contracts.md b/docs/migration/compose-orchestrator-contracts.md new file mode 100644 index 0000000000..23bf5191d3 --- /dev/null +++ b/docs/migration/compose-orchestrator-contracts.md @@ -0,0 +1,176 @@ +# Orchestrator Contract Guardrails + +> **Applies to all modules except `:feature:dashboard`.** +> Dashboard is the standalone UI hub, not an orchestrator step. +> Every other feature module is called as a step in a dynamically built step list +> and must preserve full contract compatibility with `OrchestratorFragment`. + +--- + +## How the orchestrator dispatches steps + +``` +OrchestratorViewModel.handleAction(action) + └─ BuildStepsUseCase.build(...) ← builds List from project config + └─ Step(id, navigationActionId, destinationId, params, status, result) + │ +OrchestratorFragment observes currentStep + └─ navigateSafely(actionId = step.navigationActionId, + args = step.params.toBundle()) ← params must be Parcelable/Bundle + │ +Feature Fragment/screen executes, then: + └─ finishWithResult(this, FooResult(...)) ← result via SavedStateHandle + │ +OrchestratorFragment.handleResult(FooContract.DESTINATION) { result -> + └─ orchestratorVm.handleResult(result) ← result typed as StepResult + │ +OrchestratorCache persists steps as JSON (encrypted SharedPreferences) + └─ uses orchestratorSerializersModule for polymorphic serialisation +``` + +--- + +## Guardrail 1 — Contract object: shape must not change + +```kotlin +// REQUIRED shape — do not rename, restructure, or split +object FooContract { + val DESTINATION = R.id.fooFragment // ← @IdRes; MUST stay the same Int value + fun getParams(...): FooParams = FooParams(...) // ← factory; signature may evolve + // Optional result key constants (AlertContract.ALERT_BUTTON_PRESSED_BACK pattern) +} +``` + +- `DESTINATION` references the **navigation graph fragment/composable destination ID**. + If the interop Fragment is renamed, the destination ID in `graph_foo.xml` must not change. +- `getParams()` is called by `BuildStepsUseCase` — its parameter signature may gain new + optional arguments, but must remain backwards-compatible. + +--- + +## Guardrail 2 — StepParams: serialisation annotations must be preserved exactly + +Each module's params class is cached as JSON in `OrchestratorCache` and must survive process +death and app restarts. **Any change to `@SerialName` is a cache-breaking migration.** + +```kotlin +// Mandatory annotations — all three required, none may be removed or altered +@Keep // prevents R8 from renaming the class +@Serializable // kotlinx.serialization +@SerialName("ConsentParams") // ← stable JSON discriminator; NEVER change this string +data class FooParams( + val someField: String, +) : StepParams // ← MUST implement StepParams (from :infra:core) +``` + +- `@SerialName` must equal the **original class name string** registered in + `orchestratorSerializersModule`. Changing it silently breaks deserialisation of cached steps. +- If a field is added, use `val newField: Type = defaultValue` so old cached JSON still parses. +- If a field is removed, keep it with `@Deprecated` + a default until the cache TTL has elapsed. + +--- + +## Guardrail 3 — StepResult: same rules as StepParams + +```kotlin +@Keep +@Serializable +@SerialName("FooResult") // ← never change +data class FooResult( + val someOutcome: Boolean, +) : StepResult // ← MUST implement StepResult (from :infra:core) +``` + +- Results from **all steps** are passed to `AppResponseBuilderUseCase` at the end of the flow. + Any missing or type-changed result will break response building silently. +- `ExitFormResult` and error-producing results short-circuit the flow via + `MapRefusalOrErrorResultUseCase` — this logic lives in the orchestrator and must not be + replicated in the feature module. + +--- + +## Guardrail 4 — Serialiser registration in `orchestratorSerializersModule` + +When migrating a module, verify that its `StepParams` and `StepResult` are registered in +`feature/orchestrator/src/main/java/.../steps/Step.kt`: + +```kotlin +val orchestratorSerializersModule = SerializersModule { + polymorphic(StepResult::class) { + subclass(FooResult::class) // ← must be present + } + polymorphic(StepParams::class) { + subclass(FooParams::class) // ← must be present + } +} +``` + +If a new result or params type is introduced during migration, register it here. **Do not remove +existing registrations** — old cached data may still reference them. + +--- + +## Guardrail 5 — Navigation destination ID stability + +The orchestrator navigates using `step.navigationActionId` (a nav graph action ID) and +`step.destinationId` (the destination fragment/composable ID). Both are `@IdRes Int` values +defined in XML nav graphs. + +During Compose migration using the interop Fragment shell approach: + +- **Keep `graph_foo.xml`** and the existing `` entry. +- The interop Fragment class name may change, but its nav graph declaration must keep the + same `android:id`. +- The nav graph action IDs (`R.id.action_..._to_fooFragment`) must remain stable. +- **Do not delete `graph_foo.xml`** until Navigation 3 cutover (Phase 6), which requires + coordinated updates to `BuildStepsUseCase` and `OrchestratorFragment` simultaneously. + +--- + +## Guardrail 6 — Result return mechanism: `finishWithResult()` only + +Feature modules must return results **exclusively** via: + +```kotlin +// In the interop Fragment shell +findNavController().finishWithResult(this, FooResult(...)) +``` + +- Do **not** use `Activity.setResult()` — it bypasses the `SavedStateHandle` mechanism. +- Do **not** share result state via a shared ViewModel — results must flow through the + `handleResult(FooContract.DESTINATION)` binding in `OrchestratorFragment`. +- The Composable screen should call a lambda (`onComplete: (FooResult) -> Unit`) injected by + the interop Fragment; the Fragment then calls `finishWithResult()`. + +--- + +## Guardrail 7 — `OrchestratorFragment` `handleResult` registration + +`OrchestratorFragment` has an explicit `handleResult` binding for every orchestrated step. +When adding a new step or renaming a module, verify the binding exists: + +```kotlin +// In OrchestratorFragment.onViewCreated() +handleResult(FooContract.DESTINATION, orchestratorVm::handleResult) +``` + +This line must be present for the result to reach `OrchestratorViewModel.handleResult()`. +Omitting it means the step result is silently discarded and the orchestrator stalls. + +--- + +## Per-module contract checklist + +Run this checklist before marking any module migration as complete (Stages 2–6): + +- [ ] `FooContract.DESTINATION` value unchanged +- [ ] `FooContract.getParams(...)` callable with same arguments from `BuildStepsUseCase` +- [ ] `FooParams` has `@Keep`, `@Serializable`, `@SerialName("FooParams")` — string unchanged +- [ ] `FooParams` extends `StepParams` from `:infra:core` +- [ ] `FooResult` has `@Keep`, `@Serializable`, `@SerialName("FooResult")` — string unchanged +- [ ] `FooResult` extends `StepResult` from `:infra:core` +- [ ] Both registered in `orchestratorSerializersModule` in `Step.kt` +- [ ] Nav graph XML keeps `android:id="@+id/fooFragment"` on the destination entry +- [ ] Interop Fragment calls `finishWithResult(this, FooResult(...))` — not `Activity.setResult()` +- [ ] `OrchestratorFragment` has `handleResult(FooContract.DESTINATION, orchestratorVm::handleResult)` +- [ ] `./gradlew :feature:orchestrator:test` passes after module migration diff --git a/docs/migration/compose-sim-theme.md b/docs/migration/compose-sim-theme.md new file mode 100644 index 0000000000..a596e70e83 --- /dev/null +++ b/docs/migration/compose-sim-theme.md @@ -0,0 +1,338 @@ +# SimTheme — Compose Theme Definition and Styling Guidance + +Reference document for implementing the `SimTheme` Compose theme in `:infra:compose-common`. +This translates the existing `Theme.Simprints` (Material 2, defined in `infra/resources/`) to a +single light Material 3 `MaterialTheme` wrapper and defines the styling rules screens and components should follow. + +--- + +## Styling principles + +- **3-Tier Token Hierarchy**: + 1. *Primitive Tokens*: Raw design values (`Color(0xFF00B3D1)`, `8.dp`, `16.sp`). Never use directly in screen composables. + 2. *Semantic Tokens*: Theme-level roles (`MaterialTheme.colorScheme.primary`, `MaterialTheme.typography.bodyMedium`, + `SimTheme.spacing.medium`). + 3. *Component Tokens / Styles*: Style objects or default parameter values that configure specific components (`SimButtonStyle`, + `CardDefaults.cardColors()`). +- **Immutable & Stable Tokens**: Mark custom token classes and style wrappers with `@Immutable` or `@Stable` to allow Compose compiler smart + optimizations and prevent unnecessary recompositions. +- **Custom Design Extensions**: Expose tokens outside standard M3 scales (e.g., spacing/padding, custom semantic status colors) using + `staticCompositionLocalOf` and top-level accessor extensions on `MaterialTheme` or `SimTheme`. +- **Component Slot APIs**: Keep custom UI components thin wrappers around Material 3 components. Expose parameters (`modifier`, `colors`, + `shape`, `contentPadding`, `textStyle`) instead of hardcoding styling choices. +- **Single Public Theme Entry Point**: Wrap top-level screens at the Fragment / `ComposeView` boundary using `SimTheme`. + +--- + +## Future-proofing for a Compose Styles API + +The Compose Styles API is not stable for production use. Do not add `Style`, `StyleScope`, +`Modifier.styleable()`, or related experimental APIs to this app. + +Keep components ready for a future styles API by: + +1. **Decouple Component Logic from Visual Styling**: + - Design custom components (`SimButton`, `SimCard`, `SimTextField`) to accept component style objects (e.g., `SimButtonStyle`) or style + parameters rather than querying `MaterialTheme` directly inside internal layout code. +2. **Encapsulate Style Definitions**: + - Define component style classes marked with `@Immutable` that bundle background colors, content colors, typography, shapes, and + paddings. +3. **Adapter Pattern in `SimTheme`**: + - Treat `SimTheme` as the central style provider and adapter layer. Screen call sites consume semantic component styles or default theme + values without knowing the underlying token implementation. +4. **Migration Strategy**: + - When a stable API is available and approved, adapt the internal style implementation while + preserving screen call sites and component public signatures. + +--- + +## Color mapping — `Theme.Simprints` (M2) → `SimColorScheme` (M3) + +Source files: `infra/resources/src/main/res/values/colors.xml` and `theme.xml`. + +| M2 attribute | Color name | Hex | M3 role | +|----------------------------------------|-------------------------|-------------|--------------------------------------------------| +| `colorPrimary` | `simprints_blue` | `#00B3D1` | `primary` | +| `colorPrimaryVariant` | `simprints_blue_dark` | `#009CB6` | `primaryContainer` | +| `colorOnPrimary` | `simprints_text_white` | `#DEFFFFFF` | `onPrimary` / `onPrimaryContainer` | +| `colorSecondary` / buttons | `simprints_orange` | `#FF7C00` | `secondary` | +| `colorSecondaryVariant` | `simprints_orange_dark` | `#CC6300` | `secondaryContainer` | +| `colorOnSecondary` | `simprints_text_white` | `#DEFFFFFF` | `onSecondary` / `onSecondaryContainer` | +| `android:colorBackground` | `simprints_white` | `#FFFFFF` | `background` | +| `colorSurface` | `simprints_white` | `#FFFFFF` | `surface` / `surfaceContainer` | +| `colorOnBackground` / `colorOnSurface` | `simprints_text_black` | `#DE000000` | `onBackground` / `onSurface` | +| `colorError` | `simprints_red_dark` | `#B8443F` | `error` | +| `colorOnError` | `simprints_text_white` | `#DEFFFFFF` | `onError` | +| *(no M2 equivalent)* | `simprints_green` | `#2B9962` | `tertiary` (success states) | +| status bar | `simprints_blue_dark` | `#009CB6` | drives `SystemBarStyle` via `enableEdgeToEdge()` | + +> **Company design policy exception:** This application ships exclusively with a single **Light** theme. Do **NOT** implement dark mode, and +> do **NOT** add `darkColorScheme()` or `isSystemInDarkTheme()` branching logic. + +`Color.kt`: + +```kotlin +package com.simprints.infra.composecommon.theme + +import androidx.compose.material3.lightColorScheme +import androidx.compose.runtime.Immutable +import androidx.compose.runtime.staticCompositionLocalOf +import androidx.compose.ui.graphics.Color + +// Primitive Brand Palette +val SimprintsBlue = Color(0xFF00B3D1) +val SimprintsBlueDark = Color(0xFF009CB6) +val SimprintsOrange = Color(0xFFFF7C00) +val SimprintsOrangeDark = Color(0xFFCC6300) +val SimprintsGreen = Color(0xFF2B9962) +val SimprintsRed = Color(0xFFB8443F) +val SimprintsWhite = Color(0xFFFFFFFF) +val SimprintsTextBlack = Color(0xDE000000) +val SimprintsTextWhite = Color(0xDEFFFFFF) +val SimprintsOutlineGray = Color(0x1F000000) + +val SimLightColorScheme = lightColorScheme( + primary = SimprintsBlue, + onPrimary = SimprintsTextWhite, + primaryContainer = SimprintsBlueDark, + onPrimaryContainer = SimprintsTextWhite, + secondary = SimprintsOrange, + onSecondary = SimprintsTextWhite, + secondaryContainer = SimprintsOrangeDark, + onSecondaryContainer = SimprintsTextWhite, + tertiary = SimprintsGreen, + onTertiary = SimprintsTextWhite, + error = SimprintsRed, + onError = SimprintsTextWhite, + background = SimprintsWhite, + onBackground = SimprintsTextBlack, + surface = SimprintsWhite, + onSurface = SimprintsTextBlack, + surfaceContainer = SimprintsWhite, + outline = SimprintsOutlineGray, +) + +/** + * Extended semantic colors for domain-specific states not covered by standard Material 3 roles. + */ +@Immutable +data class SimExtendedColors( + val success: Color = SimprintsGreen, + val onSuccess: Color = SimprintsTextWhite, + val warning: Color = SimprintsOrange, + val onWarning: Color = SimprintsTextWhite, +) + +val LocalSimExtendedColors = staticCompositionLocalOf { SimExtendedColors() } +``` + +--- + +## Typography mapping — `styles-text.xml` → `SimTypography` + +Source file: `infra/resources/src/main/res/values/styles-text.xml`. +Font: **Muli**, loaded from `infra/resources/src/main/res/font/muli.xml` and `muli_semibold.xml`. + +Model the app's actual text usage using full `TextStyle` definitions with explicit font family, font weight, size, line height, and letter +spacing. + +| XML `TextAppearance` | Size | M3 `Typography` slot | +|----------------------|------|----------------------| +| `Headline1` | 96sp | `displayLarge` | +| `Headline2` | 60sp | `displayMedium` | +| `Headline3` | 48sp | `displaySmall` | +| `Headline4` | 34sp | `headlineLarge` | +| `Headline5` | 24sp | `headlineMedium` | +| `Headline6` | 20sp | `headlineSmall` | +| `Subtitle1` | 16sp | `titleLarge` | +| `Subtitle2` | 14sp | `titleMedium` | +| `Body1` | 16sp | `bodyLarge` | +| `Body2` | 14sp | `bodyMedium` | +| `Button` | 14sp | `labelLarge` | +| `Caption` | 12sp | `bodySmall` | +| `Overline` | 10sp | `labelSmall` | + +`Typography.kt`: + +```kotlin +package com.simprints.infra.composecommon.theme + +import androidx.compose.material3.Typography +import androidx.compose.ui.text.TextStyle +import androidx.compose.ui.text.font.Font +import androidx.compose.ui.text.font.FontFamily +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.unit.sp + +val MuliFamily = FontFamily( + Font(com.simprints.infra.resources.R.font.muli, FontWeight.Normal), + Font(com.simprints.infra.resources.R.font.muli_semibold, FontWeight.SemiBold), +) + +val SimTypography = Typography( + displayLarge = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.Normal, fontSize = 96.sp, lineHeight = 112.sp), + displayMedium = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.Normal, fontSize = 60.sp, lineHeight = 72.sp), + displaySmall = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.Normal, fontSize = 48.sp, lineHeight = 56.sp), + headlineLarge = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.Normal, fontSize = 34.sp, lineHeight = 40.sp), + headlineMedium = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.Normal, fontSize = 24.sp, lineHeight = 32.sp), + headlineSmall = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.SemiBold, fontSize = 20.sp, lineHeight = 28.sp), + titleLarge = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.SemiBold, fontSize = 16.sp, lineHeight = 24.sp), + titleMedium = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.SemiBold, fontSize = 14.sp, lineHeight = 20.sp), + bodyLarge = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.Normal, fontSize = 16.sp, lineHeight = 24.sp), + bodyMedium = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.Normal, fontSize = 14.sp, lineHeight = 20.sp), + labelLarge = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.SemiBold, fontSize = 14.sp, lineHeight = 20.sp), + bodySmall = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.Normal, fontSize = 12.sp, lineHeight = 16.sp), + labelSmall = TextStyle(fontFamily = MuliFamily, fontWeight = FontWeight.SemiBold, fontSize = 10.sp, lineHeight = 14.sp), +) +``` + +--- + +## Shapes & Spacing mapping + +### Shapes (`Shape.kt`) + +Source file: `infra/resources/src/main/res/values/styles-shape.xml`. + +```kotlin +package com.simprints.infra.composecommon.theme + +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material3.Shapes +import androidx.compose.ui.unit.dp + +val SimShapes = Shapes( + // SmallComponent — buttons, chips, text fields + small = RoundedCornerShape(4.dp), + // MediumComponent — cards, dialogs + medium = RoundedCornerShape(8.dp), + // LargeComponent — bottom sheets, nav drawers + large = RoundedCornerShape(10.dp), +) +``` + +### Spacing & Padding (`Spacing.kt`) + +Provide layout dimension tokens via a custom `@Immutable` class and `staticCompositionLocalOf`. + +```kotlin +package com.simprints.infra.composecommon.theme + +import androidx.compose.runtime.Immutable +import androidx.compose.runtime.staticCompositionLocalOf +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp + +@Immutable +data class SimSpacing( + val extraSmall: Dp = 4.dp, + val small: Dp = 8.dp, + val medium: Dp = 16.dp, + val large: Dp = 24.dp, + val extraLarge: Dp = 32.dp, +) + +val LocalSimSpacing = staticCompositionLocalOf { SimSpacing() } +``` + +--- + +## Theme Wrapper & Accessors — `SimTheme.kt` + +`SimTheme` wraps Material 3 and injects custom CompositionLocals for spacing and extended colors. + +```kotlin +package com.simprints.infra.composecommon.theme + +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.ColorScheme +import androidx.compose.material3.Typography +import androidx.compose.material3.Shapes +import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.ReadOnlyComposable + +@Composable +fun SimTheme( + content: @Composable () -> Unit, +) { + CompositionLocalProvider( + LocalSimSpacing provides SimSpacing(), + LocalSimExtendedColors provides SimExtendedColors(), + ) { + MaterialTheme( + colorScheme = SimLightColorScheme, + typography = SimTypography, + shapes = SimShapes, + content = content, + ) + } +} + +/** + * Direct accessors for custom SimTheme extensions. + */ +object SimTheme { + val colors: ColorScheme + @Composable + @ReadOnlyComposable + get() = MaterialTheme.colorScheme + + val typography: Typography + @Composable + @ReadOnlyComposable + get() = MaterialTheme.typography + + val shapes: Shapes + @Composable + @ReadOnlyComposable + get() = MaterialTheme.shapes + + val spacing: SimSpacing + @Composable + @ReadOnlyComposable + get() = LocalSimSpacing.current + + val extendedColors: SimExtendedColors + @Composable + @ReadOnlyComposable + get() = LocalSimExtendedColors.current + +} +``` + +Usage at Fragment interop boundary: + +```kotlin +setContent { + SimTheme { + MyScreen(viewModel = hiltViewModel()) + } +} +``` + +--- + +## Edge-to-Edge & System Bar Styling + +Configure system bars at the Activity level using `enableEdgeToEdge()` with `SystemBarStyle.light()` or `SystemBarStyle.dark()` to match +`#009CB6` (`SimprintsBlueDark`). In Compose UI, rely on `WindowInsets.safeDrawing` or `WindowInsets.statusBars` rather than imperatively +mutating Activity window flags in composables. + +```kotlin +// Activity setup: +enableEdgeToEdge( + statusBarStyle = SystemBarStyle.dark( + android.graphics.Color.parseColor("#009CB6") + ) +) +``` + +--- + +## Custom Component Definition Patterns + +Build components as thin wrappers around stable Material 3 components. Use immutable parameter or +style data classes for reusable visual variants, and expose `modifier`, `colors`, `shape`, +`contentPadding`, `textStyle`, and `enabled` where appropriate. Do not use experimental Compose +Styles APIs until they are stable and explicitly adopted. diff --git a/docs/migration/compose-ui-testing.md b/docs/migration/compose-ui-testing.md new file mode 100644 index 0000000000..5b7be77508 --- /dev/null +++ b/docs/migration/compose-ui-testing.md @@ -0,0 +1,155 @@ +# Compose UI Testing with UDF State + +Reference document for testing patterns introduced during Compose migration. This migration introduces a **new UI testing approach** that +differs fundamentally from the legacy `LiveData`-based pattern used in View-based XML fragments. + +--- + +## What Changed + +| Aspect | Legacy (View + LiveData) | New (Compose + UDF) | +|-------------------------------|-------------------------------------------------------------------------|----------------------------------------------------------------| +| **State observation** | `LiveData.getOrAwaitValue()` (blocking) | `collectAsStateWithLifecycle()` (reactive collection) | +| **Effects (one-time events)** | `LiveDataEvent` wrapper + `getOrAwaitValue()` | `Flow` + `LaunchedEffect` collection | +| **Test helpers** | `InstantTaskExecutorRule` + `getOrAwaitValue()` from `infra/test-tools` | None—use standard Compose `@Composable` testing + Turbine | +| **Rendering** | Fragment layout inflation + assertions on View properties | Compose `ComposeTestRule` + `printToLog()` + semantic matchers | + +--- + +## Testing UiState in Compose + +Use **`collectAsStateWithLifecycle()`** to observe `StateFlow` in tests: + +```kotlin +@get:Rule +val composeTestRule = createComposeRule() + +@Test +fun `displays loading state initially`() { + val viewModel = MyScreenViewModel(...) // or inject via @HiltViewModel + testRule + + composeTestRule.setContent { + SimTheme { + val uiState = viewModel.uiState.collectAsStateWithLifecycle().value + MyScreen(uiState = uiState, onEvent = {}) + } + } + + // Assert initial state + composeTestRule.onNodeWithText("Loading...").assertIsDisplayed() + + // Trigger action + composeTestRule.onNodeWithContentDescription("Retry").performClick() + + // Wait for new state and assert + composeTestRule.waitUntil(timeoutMillis = 5000) { + composeTestRule.onAllNodesWithText("Success").fetchSemanticsNodes().isNotEmpty() + } +} +``` + +--- + +## Testing UiEffect (One-Time Events) + +Use **`LaunchedEffect`** to collect and verify effects: + +```kotlin +@Test +fun `navigates when login fails`() { + val viewModel = LoginViewModel(...) + val capturedEffects = mutableListOf() + + composeTestRule.setContent { + SimTheme { + LaunchedEffect(Unit) { + viewModel.uiEffect.collect { effect -> + capturedEffects.add(effect) + } + } + val uiState = viewModel.uiState.collectAsStateWithLifecycle().value + LoginScreen(uiState = uiState, onIntent = viewModel::onIntent) + } + } + + // Trigger invalid login + composeTestRule.onNodeWithContentDescription("Email").performTextInput("invalid@") + composeTestRule.onNodeWithText("Sign In").performClick() + + // Assert effect was emitted + composeTestRule.waitUntil(timeoutMillis = 5000) { + capturedEffects.any { it is LoginUiEffect.ShowError } + } + Truth.assertThat(capturedEffects).hasSize(1) +} +``` + +--- + +## Testing with Turbine (Advanced) + +For **unit tests of ViewModel logic** (not Compose UI rendering), use **[Turbine](https://github.com/cashapp/turbine)** to assert Flow +emissions: + +```kotlin +@Test +fun `reducer updates state on login success`() = runTest { + val repository = FakeUserRepository() + val viewModel = LoginViewModel(repository) + + turbineScope { + val stateTurbine = viewModel.uiState.testIn(backgroundScope) + val effectsTurbine = viewModel.uiEffect.testIn(backgroundScope) + + // Initial state + Truth.assertThat(stateTurbine.awaitItem()).isEqualTo(LoginUiState()) + + // Trigger login + viewModel.onIntent(LoginUiAction.SignInClicked("user@example.com", "password")) + + // Verify state progression + Truth.assertThat(stateTurbine.awaitItem().isLoading).isTrue() + Truth.assertThat(stateTurbine.awaitItem().isLoading).isFalse() + + // Verify effect + Truth.assertThat(effectsTurbine.awaitItem()) + .isInstanceOf(LoginUiEffect.NavigateToHome::class.java) + } + } +``` + +--- + +## Key Differences from Legacy Testing + +1. **No `InstantTaskExecutorRule` needed** — `StateFlow` and effect `Flow`s are not tied to a + `LiveData` executor. +2. **No `getOrAwaitValue()` helper** — state is immutable; just read `.value` synchronously after collection. +3. **Effect transport follows the UDF guide** — use `MutableSharedFlow(replay = 0)` by default. + Use `Channel` only when strict single-consumer FIFO queue semantics are required. Tests must + collect the public `Flow` regardless of the selected transport. +4. **Compose test rule is required** — you must compose the screen to trigger recompositions and verify UI. +5. **Effects collection must happen in a `LaunchedEffect`** — this ensures the collector is lifecycle-aware and survives configuration + changes during the test. + +--- + +## Migration Checklist for ViewModel Tests + +When migrating a ViewModel's tests from `LiveData` to UDF: + +- [ ] Replace `InstantTaskExecutorRule` with `ComposeTestRule` or `runTest { }` block +- [ ] Replace `liveData.getOrAwaitValue()` with state Flow collection via `collectAsStateWithLifecycle()` (Compose tests) or Turbine (unit + tests) +- [ ] Replace `LiveDataEvent` assertions with `Flow` collection in `LaunchedEffect` +- [ ] Remove old `LiveData` test helpers; use Turbine for Flow assertions +- [ ] Add `@get:Rule val composeTestRule = createComposeRule()` to Compose UI tests +- [ ] Verify navigation effects are tested via orchestrator result handling, not direct effect assertion +- [ ] Run full test suite to ensure no timing issues (use `waitUntil { }` for async state changes) + +--- + +**Related references:** + +- See [compose-viewmodel-udf.md](compose-viewmodel-udf.md#2f--testing-the-mvi-contract) for the + canonical ViewModel UDF testing guide and effect-transport decision. diff --git a/docs/migration/compose-viewmodel-udf.md b/docs/migration/compose-viewmodel-udf.md new file mode 100644 index 0000000000..fe4f1f3ce7 --- /dev/null +++ b/docs/migration/compose-viewmodel-udf.md @@ -0,0 +1,353 @@ +# ViewModel Modernisation — MVI with Kotlin Flow + +Reference document for Phase 2 of the Compose migration. Standardises migrated ViewModels on a +single **MVI-style UDF contract** and replaces scattered `LiveData`, `LiveDataEvent`, +`LiveDataEventWithContent`, and ad-hoc navigation LiveData patterns. + +--- + +## MVI contract + +This keeps all user/system actions explicit (`UiAction`), all render state immutable (`UiState`), +and all one-off side effects (`UiEffect`) separated from state. + +--- + +## 2a — Define `UiState`, `UiAction`, `UiEffect` per screen + +Each screen gets three co-located contracts: + +```kotlin +internal data class MyScreenUiState( + val isLoading: Boolean = false, + val projectConfiguration: ProjectConfigurationUi? = null, + val error: ErrorUi? = null, +) { + internal data class ProjectConfigurationUi( + val projectId: String, + val projectName: String, + val language: String, + ) + + internal data class ErrorUi( + val message: String, + val isRetryable: Boolean, + ) +} + +internal sealed interface MyScreenUiAction { + data object InitialLoad : MyScreenUiAction + data object RetryClicked : MyScreenUiAction + data object ContinueClicked : MyScreenUiAction +} + +internal sealed interface MyScreenUiEffect { + data object NavigateToLogin : MyScreenUiEffect + data class ShowErrorSnackbar(val message: String) : MyScreenUiEffect + data class NavigateToConsent(val params: ConsentParams) : MyScreenUiEffect +} +``` + +**Rules:** + +- `UiState` is always an immutable data class with defaults. +- `UiAction` is the only entry point from UI into ViewModel. +- `UiEffect` contains one-time side effects only; never put render state in effects. +- Keep contracts `internal` unless part of a module public API. + +--- + +## 2b — Base Kotlin Flow MVI ViewModel (recommended default) + +Use a small base class to remove boilerplate and enforce the same flow in every module. + +```kotlin +internal abstract class BaseMviViewModel( + initialState: UiState, +) : ViewModel() { + + private val _uiState = MutableStateFlow(initialState) + val uiState: StateFlow = _uiState.asStateFlow() + + // Default effect stream: no replay, buffered, supports Compose collectors. + private val _uiEffect = MutableSharedFlow( + replay = 0, + extraBufferCapacity = 16, + ) + val uiEffect: Flow = _uiEffect.asSharedFlow() + + fun onIntent(intent: UiAction) { + viewModelScope.launch { + handleIntent(intent) + } + } + + protected abstract suspend fun handleIntent(intent: UiAction) + + protected fun reduce(reducer: (UiState) -> UiState) { + _uiState.update(reducer) + } + + protected suspend fun emitEffect(effect: UiEffect) { + _uiEffect.emit(effect) + } +} +``` + +### Effect transport choice + +- **Default:** `MutableSharedFlow` (`replay = 0`) for Compose-first UIs. +- **Use `Channel`** only when strict single-consumer FIFO queue semantics are required. + +```kotlin +private val _uiEffect = Channel(capacity = Channel.BUFFERED) +val uiEffect: Flow = _uiEffect.receiveAsFlow() +``` + +--- + +## 2c — Screen ViewModel implementation pattern + +This sample reflects a common Simprints flow where the screen bootstraps by loading +project configuration from local storage before allowing the user to continue. + +```kotlin +@HiltViewModel +internal class MyScreenViewModel @Inject constructor( + private val projectConfigurationStore: ProjectConfigurationStore, + private val savedStateHandle: SavedStateHandle, +) : BaseMviViewModel( + initialState = MyScreenUiState( + projectConfiguration = savedStateHandle.get("project_id")?.let { projectId -> + MyScreenUiState.ProjectConfigurationUi( + projectId = projectId, + ) + }, + ), +) { + + init { + if (uiState.value.projectConfiguration == null) { + onIntent(MyScreenUiAction.InitialLoad) + } + } + + override suspend fun handleIntent(intent: MyScreenUiAction) { + when (intent) { + MyScreenUiAction.InitialLoad, + MyScreenUiAction.RetryClicked -> loadProjectConfiguration() + MyScreenUiAction.ContinueClicked -> { + val config = uiState.value.projectConfiguration + if (config == null) { + emitEffect(MyScreenUiEffect.ShowErrorSnackbar("Project configuration unavailable")) + return + } + emitEffect( + MyScreenUiEffect.NavigateToConsent( + ConsentContract.getParams(projectId = config.projectId), + ), + ) + } + } + } + + private suspend fun loadProjectConfiguration() { + reduce { it.copy(isLoading = true, error = null) } + + val configuration = projectConfigurationStore.getProjectConfiguration() + if (configuration != null) { + savedStateHandle["project_id"] = configuration.projectId + reduce { + it.copy( + isLoading = false, + projectConfiguration = MyScreenUiState.ProjectConfigurationUi( + projectId = configuration.projectId, + ), + ) + } + } else { + Simber.i("Project configuration not found in local storage") + reduce { + it.copy( + isLoading = false, + error = MyScreenUiState.ErrorUi( + message = "Unable to load project configuration", + isRetryable = true, + ), + ) + } + emitEffect(MyScreenUiEffect.ShowErrorSnackbar("Unable to load project configuration")) + } + } +} +``` + +**Key rules:** + +- Keep `MutableStateFlow` and `MutableSharedFlow`/`Channel` private. +- Update state via reducers (`update`/`reduce`), not direct mutable UI fields. +- If values are persisted in `SavedStateHandle`, restore them in `initialState`. +- Do not expose mutable collections in `UiState`; expose immutable snapshots. +- Keep logging through `Simber`. + +--- + +## 2d — Migration map from existing patterns + +| Current pattern | Replace with | +|----------------------------------------------------------|--------------------------------------------------------------------------------| +| `MutableLiveData` + backing field | `private val _uiState = MutableStateFlow(UiState())` | +| Public `LiveData` | `val uiState: StateFlow` | +| `LiveDataEvent` / `LiveDataEventWithContent` | `UiEffect` on `MutableSharedFlow(replay = 0)` (or `Channel`) | +| `MutableLiveData` | `UiEffect.Navigate(params)` | +| Fragment callback methods like `onRetry()` / `onClick()` | `onIntent(UiAction.RetryClicked)` / `onIntent(UiAction.X)` | +| Multiple LiveData fields | One immutable `UiState` | +| `flow.asLiveData(...)` | `flow.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), initial)` | +| Fragment `observe {}` | Compose `collectAsStateWithLifecycle()` | +| Fragment lifecycle collection for one-time events | `LaunchedEffect(viewModel) { viewModel.uiEffect.collect { ... } }` | + +--- + +## 2e — Compose collection patterns + +```kotlin +@Composable +internal fun MyScreen( + viewModel: MyScreenViewModel = hiltViewModel(), + onNavigateToLogin: () -> Unit, + onNavigateToConsent: (ConsentParams) -> Unit, +) { + val state by viewModel.uiState.collectAsStateWithLifecycle() + + LaunchedEffect(viewModel) { + viewModel.uiEffect.collect { effect -> + when (effect) { + MyScreenUiEffect.NavigateToLogin -> onNavigateToLogin() + is MyScreenUiEffect.ShowErrorSnackbar -> { + // trigger snackbar host state + } + is MyScreenUiEffect.NavigateToConsent -> { + onNavigateToConsent(effect.params) + } + } + } + } + + MyScreenContent( + state = state, + onRetry = { viewModel.onIntent(MyScreenUiAction.RetryClicked) }, + onContinue = { viewModel.onIntent(MyScreenUiAction.ContinueClicked) }, + ) +} +``` + +Use `collectAsStateWithLifecycle()` for render state and `LaunchedEffect(viewModel)` for effects. + +--- + +## 2f — Testing the MVI contract + +```kotlin +@Test +fun `initial load fetches project configuration`() = runTest { + every { projectConfigurationStore.getProjectConfiguration() } returns ProjectConfiguration( + projectId = "project-123", + ) + + viewModel.onIntent(MyScreenUiAction.InitialLoad) + + assertThat(viewModel.uiState.value.projectConfiguration?.projectId).isEqualTo("project-123") + assertThat(viewModel.uiState.value.isLoading).isFalse() + } + +@Test +fun `continue emits consent navigation effect`() = runTest { + every { projectConfigurationStore.getProjectConfiguration() } returns ProjectConfiguration( + projectId = "project-123", + ) + + viewModel.uiEffect.test { + viewModel.onIntent(MyScreenUiAction.InitialLoad) + viewModel.onIntent(MyScreenUiAction.ContinueClicked) + assertThat(awaitItem()).isInstanceOf(MyScreenUiEffect.NavigateToConsent::class.java) + cancelAndIgnoreRemainingEvents() + } +} +``` + +Add Turbine where needed: + +```kotlin +testImplementation(libs.turbine) +``` + +Keep `TestCoroutineRule`. Keep `InstantTaskExecutorRule` only while LiveData remains in that module. + +--- + +## 2g — Migrating complex Flow ViewModels (`SyncInfoViewModel` style) + +When a ViewModel already composes multiple flows, keep transformation in Flow and expose a single +hot `StateFlow`: + +```kotlin +val uiState: StateFlow = mergedFlow + .map { syncInfo -> syncInfo.toUiState() } + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = SyncInfoUiState(), + ) +``` + +`SharingStarted.WhileSubscribed(5_000)` is the default for screen state because it survives short +collector gaps (e.g., transient lifecycle changes) without keeping upstream alive indefinitely. + +--- + +## 2h — Shared ViewModels (screen + workflow split) + +For flows like biometric capture, use two ViewModels with strict boundaries: + +- **Screen-scoped capture ViewModel**: camera permission/session, preview UI state, quality hints, + capture button enablement, and one-time capture effects. +- **Shared workflow ViewModel** (activity or nav-graph scoped): template extraction, retries, + progress, orchestration decisions, and final result state. + +Do not inject one ViewModel into another. Bridge them in the host UI: + +```kotlin +@Composable +internal fun CaptureRoute( + captureViewModel: CaptureViewModel = hiltViewModel(), + workflowViewModel: CaptureWorkflowViewModel, +) { + val captureState by captureViewModel.uiState.collectAsStateWithLifecycle() + + LaunchedEffect(captureViewModel) { + captureViewModel.uiEffect.collect { effect -> + when (effect) { + is CaptureUiEffect.CaptureCompleted -> { + workflowViewModel.onIntent( + CaptureWorkflowUiAction.ExtractTemplate( + imageBytes = effect.imageBytes, + metadata = effect.metadata, + ), + ) + } + } + } + } + + CaptureScreen( + state = captureState, + onCapture = { captureViewModel.onIntent(CaptureUiAction.CaptureClicked) }, + ) +} +``` + +Scoping guidance: + +- Prefer **nav-graph scoped shared ViewModels** when the workflow lifetime is tied to one graph. +- Use **activity scope** only when the workflow must span multiple graphs/screens. +- Keep resume-critical workflow values in `SavedStateHandle` on the shared workflow ViewModel. From 01450d8af88269b0f8565b7af2861be5b931f363 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 3 Aug 2026 16:40:24 +0300 Subject: [PATCH 2/3] MS-1440 Metric collection scripts --- .gitignore | 1 + .../metrics/compose-migration-metrics.md | 96 +++++++++ .../metrics/compose-module-status.csv | 19 ++ .../collect-compose-migration-metrics.py | 184 ++++++++++++++++++ .../metrics/scripts/gradle-profiler.scenarios | 20 ++ .../scripts/parse-gradle-profiler-csv.py | 89 +++++++++ .../metrics/scripts/record-metric-fragment.py | 57 ++++++ .../scripts/run-gradle-profiler-metrics.sh | 69 +++++++ 8 files changed, 535 insertions(+) create mode 100644 docs/migration/metrics/compose-migration-metrics.md create mode 100644 docs/migration/metrics/compose-module-status.csv create mode 100755 docs/migration/metrics/scripts/collect-compose-migration-metrics.py create mode 100644 docs/migration/metrics/scripts/gradle-profiler.scenarios create mode 100755 docs/migration/metrics/scripts/parse-gradle-profiler-csv.py create mode 100755 docs/migration/metrics/scripts/record-metric-fragment.py create mode 100755 docs/migration/metrics/scripts/run-gradle-profiler-metrics.sh diff --git a/.gitignore b/.gitignore index cdfb331247..dcd1c43d9e 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,7 @@ build/ projectFilesBackup/ .kotlin/ !.idea/ktlint-plugin.xml +gradle-user-home # PyCharm testing/__pycache__/ diff --git a/docs/migration/metrics/compose-migration-metrics.md b/docs/migration/metrics/compose-migration-metrics.md new file mode 100644 index 0000000000..50a5752093 --- /dev/null +++ b/docs/migration/metrics/compose-migration-metrics.md @@ -0,0 +1,96 @@ +# Compose migration metrics collection runbook + +This document defines a repeatable process to collect all migration metrics into one place: + +- `docs/migration/metrics/results/latest.json` (latest snapshot) +- `docs/migration/metrics/results/history.ndjson` (time-series history) + +## 1. One-time setup + +1. Fill `docs/migration/metrics/compose-module-status.csv` with one row per `:feature:*` module. +2. Install local tools: + - `gradle-profiler` (for build time metrics) + - `bundletool` (for download-size metrics) +3. Keep this baseline commit SHA (last XML-only baseline) in your notes. You will compare new snapshots against that baseline row in + `history.ndjson`. + +## 2. Metrics and where they come from + +| Metric | Source | Automated by | +|--------------------------------|----------------------------------------------------------------------------|-------------------------------------------------------------------| +| Module Conversion % | `docs/migration/metrics/compose-module-status.csv` + `settings.gradle.kts` | `collect-compose-migration-metrics.py` | +| View-Compose Interop Nodes | Kotlin source scan (`ComposeView`, `AndroidView`) | `collect-compose-migration-metrics.py` | +| Code Ignored in Coverage | `@ExcludedFromGeneratedTestCoverageReports` usage + optional JaCoCo XML | `collect-compose-migration-metrics.py` | +| Build Time (clean/incremental) | `gradle-profiler` benchmark CSV | `run-gradle-profiler-metrics.sh` + `parse-gradle-profiler-csv.py` | +| APK Download Size | AAB artifact size | `record-metric-fragment.py` | + +## 3. Step-by-step collection + +### Step 1 - Collect static metrics + +```bash +python3 docs/migration/metrics/scripts/collect-compose-migration-metrics.py +``` + +This creates/updates: + +- `docs/migration/metrics/results/latest.json` +- `docs/migration/metrics/results/history.ndjson` + +### Step 2 - Collect build time metrics (clean + incremental) + +Run gradle-profiler: + +```bash +bash docs/migration/metrics/scripts/run-gradle-profiler-metrics.sh +``` + +This writes a fragment: + +- `docs/migration/metrics/results/fragments/build_time.json` + +### Step 3 - Collect APK size metrics + +Build the bundle: + +```bash +./gradlew id:bundleDebug +``` + +Record bundle size (bytes) as a metric fragment: + +```bash +python3 docs/migration/metrics/scripts/record-metric-fragment.py \ + docs/migration/metrics/results/fragments/apk_size.json \ + apk_size.aab_bytes=$(stat -f%z id/build/outputs/bundle/debug/id-debug.aab) +``` + +### Step 4 - Merge it into the unified snapshot: + +```bash +python3 docs/migration/metrics/scripts/collect-compose-migration-metrics.py +``` + +## 4. Keeping metrics comparable + +1. Always run against the same build variant (`debug` or dedicated `benchmark`) and keep it fixed. +2. Use the same device class/OS image for runtime performance. +3. Compare each new row against the baseline row from the XML-only commit. +4. Do not change module status semantics: + - `legacy`: XML/View system + - `interop`: mixed View/Compose + - `compose`: fully Compose + +## 5. Script quick reference + +- `docs/migration/metrics/scripts/collect-compose-migration-metrics.py` + Collects static metrics and merges all metric fragments into one snapshot/history. + +- `docs/migration/metrics/scripts/run-gradle-profiler-metrics.sh` + Runs gradle-profiler scenarios and emits `build_time.json`. + +- `docs/migration/metrics/scripts/parse-gradle-profiler-csv.py` + Parses `benchmark.csv` and computes average scenario times in seconds. + +- `docs/migration/metrics/scripts/record-metric-fragment.py` + Writes/updates a JSON fragment from `key=value` arguments for manual or scripted ingestion. diff --git a/docs/migration/metrics/compose-module-status.csv b/docs/migration/metrics/compose-module-status.csv new file mode 100644 index 0000000000..f52b53a283 --- /dev/null +++ b/docs/migration/metrics/compose-module-status.csv @@ -0,0 +1,19 @@ +module,status,migrated_on,notes +:feature:orchestrator,legacy,, +:feature:client-api,legacy,, +:feature:login-check,legacy,, +:feature:login,legacy,, +:feature:fetch-subject,legacy,, +:feature:select-subject,legacy,, +:feature:enrol-last-biometric,legacy,, +:feature:external-credential,legacy,, +:feature:dashboard,legacy,, +:feature:troubleshooting,legacy,, +:feature:alert,legacy,, +:feature:exit-form,legacy,, +:feature:consent,legacy,, +:feature:setup,legacy,, +:feature:matcher,legacy,, +:feature:validate-subject-pool,legacy,, +:feature:select-subject-age-group,legacy,, +:feature:storage-alert,legacy,, diff --git a/docs/migration/metrics/scripts/collect-compose-migration-metrics.py b/docs/migration/metrics/scripts/collect-compose-migration-metrics.py new file mode 100755 index 0000000000..e116afd755 --- /dev/null +++ b/docs/migration/metrics/scripts/collect-compose-migration-metrics.py @@ -0,0 +1,184 @@ +#!/usr/bin/env python3 +"""Collect Compose migration metrics and write a unified snapshot/history.""" + +from __future__ import annotations + +import argparse +import csv +import datetime as dt +import json +import re +import subprocess +from pathlib import Path +from typing import Any + +FEATURE_PATTERN = re.compile(r'":feature:[^"]+"') +INTEROP_PATTERNS = { + "compose_view": re.compile(r"\bComposeView\b"), + "android_view": re.compile(r"\bAndroidView\b"), +} +EXCLUDED_COVERAGE_PATTERN = re.compile(r"@ExcludedFromGeneratedTestCoverageReports\b") + + +def read_text(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def git_value(repo_root: Path, args: list[str]) -> str | None: + try: + result = subprocess.run( + ["git", *args], + cwd=repo_root, + capture_output=True, + text=True, + check=True, + ) + return result.stdout.strip() or None + except Exception: + return None + + +def parse_feature_modules(settings_path: Path) -> list[str]: + content = read_text(settings_path) + modules = sorted(set(m.strip('"') for m in FEATURE_PATTERN.findall(content))) + return modules + + +def parse_status_csv(status_csv_path: Path) -> dict[str, str]: + if not status_csv_path.exists(): + return {} + status_by_module: dict[str, str] = {} + with status_csv_path.open("r", encoding="utf-8", newline="") as f: + reader = csv.DictReader(f) + for row in reader: + module = (row.get("module") or "").strip() + status = (row.get("status") or "").strip().lower() + if module: + status_by_module[module] = status + return status_by_module + + +def count_in_files(repo_root: Path, pattern: re.Pattern[str], suffix: str) -> int: + count = 0 + for path in repo_root.rglob(f"*{suffix}"): + if any(part in {"build", ".git", ".gradle"} for part in path.parts): + continue + try: + count += len(pattern.findall(path.read_text(encoding="utf-8", errors="ignore"))) + except OSError: + continue + return count + + +def load_fragments(fragment_dir: Path) -> dict[str, Any]: + merged: dict[str, Any] = {} + if not fragment_dir.exists(): + return merged + for fragment_path in sorted(fragment_dir.glob("*.json")): + try: + data = json.loads(fragment_path.read_text(encoding="utf-8")) + if isinstance(data, dict): + merged.update(data) + except json.JSONDecodeError: + continue + return merged + + +def build_metrics(repo_root: Path, status_csv: Path, fragment_dir: Path) -> dict[str, Any]: + settings_path = repo_root / "settings.gradle.kts" + feature_modules = parse_feature_modules(settings_path) + status_by_module = parse_status_csv(status_csv) + + migrated_modules = [ + module + for module in feature_modules + if status_by_module.get(module) in {"compose", "migrated"} + ] + + interop_counts = { + name: count_in_files(repo_root, pattern, ".kt") + for name, pattern in INTEROP_PATTERNS.items() + } + interop_total = interop_counts["compose_view"] + interop_counts["android_view"] + + excluded_coverage_annotation_count = count_in_files( + repo_root, EXCLUDED_COVERAGE_PATTERN, ".kt" + ) + + total_feature_modules = len(feature_modules) + conversion_pct = ( + (len(migrated_modules) / total_feature_modules) * 100.0 + if total_feature_modules > 0 + else 0.0 + ) + + now = dt.datetime.now(dt.timezone.utc).isoformat() + commit = git_value(repo_root, ["rev-parse", "HEAD"]) + branch = git_value(repo_root, ["rev-parse", "--abbrev-ref", "HEAD"]) + + metrics: dict[str, Any] = { + "timestamp_utc": now, + "git.commit": commit, + "git.branch": branch, + "module.total_feature_modules": total_feature_modules, + "module.migrated_feature_modules": len(migrated_modules), + "module.conversion_pct": round(conversion_pct, 2), + "interop.compose_view_nodes": interop_counts["compose_view"], + "interop.android_view_nodes": interop_counts["android_view"], + "interop.total_nodes": interop_total, + "coverage.excluded_annotation_count": excluded_coverage_annotation_count, + } + + metrics.update(load_fragments(fragment_dir)) + return metrics + + +def append_history(history_path: Path, row: dict[str, Any]) -> None: + history_path.parent.mkdir(parents=True, exist_ok=True) + with history_path.open("a", encoding="utf-8") as f: + f.write(json.dumps(row, sort_keys=True)) + f.write("\n") + + +def write_latest(latest_path: Path, row: dict[str, Any]) -> None: + latest_path.parent.mkdir(parents=True, exist_ok=True) + latest_path.write_text(json.dumps(row, indent=2, sort_keys=True) + "\n", encoding="utf-8") + + +def main() -> None: + parser = argparse.ArgumentParser() + parser.add_argument("--repo-root", default=".") + parser.add_argument( + "--status-csv", + default="docs/migration/metrics/compose-module-status.csv", + ) + parser.add_argument( + "--fragment-dir", + default="docs/migration/metrics/results/fragments", + ) + parser.add_argument( + "--latest-output", + default="docs/migration/metrics/results/latest.json", + ) + parser.add_argument( + "--history-output", + default="docs/migration/metrics/results/history.ndjson", + ) + args = parser.parse_args() + + repo_root = Path(args.repo_root).resolve() + status_csv = (repo_root / args.status_csv).resolve() + fragment_dir = (repo_root / args.fragment_dir).resolve() + latest_output = (repo_root / args.latest_output).resolve() + history_output = (repo_root / args.history_output).resolve() + + metrics = build_metrics(repo_root, status_csv, fragment_dir) + write_latest(latest_output, metrics) + append_history(history_output, metrics) + + print(f"Wrote {latest_output}") + print(f"Appended {history_output}") + + +if __name__ == "__main__": + main() diff --git a/docs/migration/metrics/scripts/gradle-profiler.scenarios b/docs/migration/metrics/scripts/gradle-profiler.scenarios new file mode 100644 index 0000000000..e77ea3a272 --- /dev/null +++ b/docs/migration/metrics/scripts/gradle-profiler.scenarios @@ -0,0 +1,20 @@ +default-scenarios = ["clean_build", "incremental_build"] + +clean_build { + title = "Clean build" + tasks = ["id:assembleDebug"] + cleanup-tasks = ["clean"] + daemon = warm + warm-ups = 1 + iterations = 1 +} + +incremental_build { + title = "Incremental build" + tasks = ["id:assembleDebug"] + daemon = warm + warm-ups = 1 + iterations = 1 + + apply-android-layout-change-to = "feature/dashboard/src/main/res/layout/fragment_main.xml" +} diff --git a/docs/migration/metrics/scripts/parse-gradle-profiler-csv.py b/docs/migration/metrics/scripts/parse-gradle-profiler-csv.py new file mode 100755 index 0000000000..9312022ad7 --- /dev/null +++ b/docs/migration/metrics/scripts/parse-gradle-profiler-csv.py @@ -0,0 +1,89 @@ +#!/usr/bin/env python3 +"""Parse gradle-profiler benchmark CSV and emit build_time metrics fragment JSON.""" + +from __future__ import annotations + +import argparse +import csv +import json +from pathlib import Path + + +SCENARIO_COLUMNS = ("scenario", "benchmark", "name") +VALUE_COLUMNS = ("value", "total", "duration", "time", "mean", "median") +UNIT_COLUMNS = ("unit", "units") + + +def first_existing(header: list[str], candidates: tuple[str, ...]) -> str | None: + lowered = {h.lower(): h for h in header} + for candidate in candidates: + if candidate in lowered: + return lowered[candidate] + return None + + +def to_seconds(value: float, unit: str | None) -> float: + if unit is None: + return value + normalized = unit.strip().lower() + if normalized in {"s", "sec", "secs", "second", "seconds"}: + return value + if normalized in {"ms", "millisecond", "milliseconds"}: + return value / 1000.0 + if normalized in {"ns", "nanosecond", "nanoseconds"}: + return value / 1_000_000_000.0 + return value + + +def parse(csv_path: Path) -> dict[str, float]: + values_by_scenario: dict[str, list[float]] = {} + with csv_path.open("r", encoding="utf-8", newline="") as f: + reader = csv.DictReader(f) + if not reader.fieldnames: + return {} + scenario_col = first_existing(reader.fieldnames, SCENARIO_COLUMNS) + value_col = first_existing(reader.fieldnames, VALUE_COLUMNS) + unit_col = first_existing(reader.fieldnames, UNIT_COLUMNS) + if scenario_col is None or value_col is None: + return {} + + for row in reader: + scenario = (row.get(scenario_col) or "").strip() + raw_value = (row.get(value_col) or "").strip() + if not scenario or not raw_value: + continue + try: + value = float(raw_value) + except ValueError: + continue + unit = (row.get(unit_col) or "").strip() if unit_col else None + values_by_scenario.setdefault(scenario, []).append(to_seconds(value, unit)) + + metrics: dict[str, float] = {} + for scenario, values in values_by_scenario.items(): + if not values: + continue + avg = sum(values) / len(values) + safe_name = scenario.strip().lower().replace(" ", "_") + metrics[f"build_time.{safe_name}_seconds_avg"] = round(avg, 4) + metrics[f"build_time.{safe_name}_sample_count"] = float(len(values)) + return metrics + + +def main() -> None: + parser = argparse.ArgumentParser() + parser.add_argument("csv_path") + parser.add_argument("output_json") + args = parser.parse_args() + + csv_path = Path(args.csv_path).resolve() + output_json = Path(args.output_json).resolve() + output_json.parent.mkdir(parents=True, exist_ok=True) + + metrics = parse(csv_path) + output_json.write_text(json.dumps(metrics, indent=2, sort_keys=True) + "\n", encoding="utf-8") + print(f"Wrote {output_json}") + + +if __name__ == "__main__": + main() diff --git a/docs/migration/metrics/scripts/record-metric-fragment.py b/docs/migration/metrics/scripts/record-metric-fragment.py new file mode 100755 index 0000000000..18f4952bbc --- /dev/null +++ b/docs/migration/metrics/scripts/record-metric-fragment.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +"""Write or update a metric fragment JSON from key=value arguments.""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path + + +def parse_value(raw: str): + lowered = raw.lower() + if lowered == "true": + return True + if lowered == "false": + return False + try: + if "." in raw: + return float(raw) + return int(raw) + except ValueError: + return raw + + +def main() -> None: + parser = argparse.ArgumentParser() + parser.add_argument("output_json") + parser.add_argument("pairs", nargs="+") + args = parser.parse_args() + + output_json = Path(args.output_json).resolve() + output_json.parent.mkdir(parents=True, exist_ok=True) + + current = {} + if output_json.exists(): + try: + current = json.loads(output_json.read_text(encoding="utf-8")) + if not isinstance(current, dict): + current = {} + except json.JSONDecodeError: + current = {} + + for pair in args.pairs: + if "=" not in pair: + raise ValueError(f"Expected key=value format, got: {pair}") + key, raw_value = pair.split("=", 1) + key = key.strip() + if not key: + raise ValueError(f"Metric key cannot be blank: {pair}") + current[key] = parse_value(raw_value.strip()) + + output_json.write_text(json.dumps(current, indent=2, sort_keys=True) + "\n", encoding="utf-8") + print(f"Wrote {output_json}") + + +if __name__ == "__main__": + main() diff --git a/docs/migration/metrics/scripts/run-gradle-profiler-metrics.sh b/docs/migration/metrics/scripts/run-gradle-profiler-metrics.sh new file mode 100755 index 0000000000..994f14ba55 --- /dev/null +++ b/docs/migration/metrics/scripts/run-gradle-profiler-metrics.sh @@ -0,0 +1,69 @@ +#!/usr/bin/env bash + +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "$0")/../../../.." && pwd)" +echo "Root: $ROOT_DIR" + +SCENARIO_FILE="$ROOT_DIR/docs/migration/metrics/scripts/gradle-profiler.scenarios" + +# Intermediate raw profiler output goes in build/ (ephemeral, not committed). +INTERMEDIATE_DIR="${1:-$ROOT_DIR/docs/build/migration-metrics/gradle-profiler}" +echo "Intermediate Dir: $INTERMEDIATE_DIR" + +# Final parsed fragment is persisted in docs/migration/metrics/results/. +FRAGMENT_OUT="$ROOT_DIR/docs/migration/metrics/results/fragments/build_time.json" + +if ! command -v gradle-profiler >/dev/null 2>&1; then + echo "ERROR: gradle-profiler is not installed or not on PATH" >&2 + echo "Install with: brew install gradle-profiler" >&2 + exit 1 +fi + +GIT_COMMIT="$(git -C "$ROOT_DIR" rev-parse HEAD 2>/dev/null || echo "unknown")" +GIT_BRANCH="$(git -C "$ROOT_DIR" rev-parse --abbrev-ref HEAD 2>/dev/null || echo "unknown")" + +echo "Recording build-time metrics for commit=$GIT_COMMIT branch=$GIT_BRANCH" + +# gradle-profiler requires the output directory to not exist — it creates it +# itself. Remove any leftover from a previous run before proceeding. +rm -rf "$INTERMEDIATE_DIR" +mkdir -p "$INTERMEDIATE_DIR" + +gradle-profiler \ + --benchmark \ + --csv-format long \ + --project-dir "$ROOT_DIR" \ + --scenario-file "$SCENARIO_FILE" \ + --output-dir "$INTERMEDIATE_DIR" + +CSV_PATH="$INTERMEDIATE_DIR/benchmark.csv" +echo "CSV_PATH Dir: $CSV_PATH" + +# +#if [ ! -f "$CSV_PATH" ]; then +# # gradle-profiler sometimes creates a versioned subdirectory when the target +# # already exists; fall back to finding the CSV anywhere under INTERMEDIATE_DIR. +# CSV_PATH="$(find "$INTERMEDIATE_DIR" -name "benchmark.csv" -maxdepth 2 | head -n 1)" +#fi +#if [ -z "$CSV_PATH" ] || [ ! -f "$CSV_PATH" ]; then +# echo "ERROR: benchmark.csv not found under $INTERMEDIATE_DIR" >&2 +# exit 1 +#fi + +# Parse CSV into metric keys and write initial fragment. +python3 "$ROOT_DIR/docs/migration/metrics/scripts/parse-gradle-profiler-csv.py" \ + "$CSV_PATH" \ + "$FRAGMENT_OUT" + +# Stamp the fragment with the git coordinates so it can be correlated +# with snapshots from collect-compose-migration-metrics.py. +python3 "$ROOT_DIR/docs/migration/metrics/scripts/record-metric-fragment.py" \ + "$FRAGMENT_OUT" \ + "build_time.git_commit=$GIT_COMMIT" \ + "build_time.git_branch=$GIT_BRANCH" + +echo "Build-time fragment written to $FRAGMENT_OUT" +echo " git.commit : $GIT_COMMIT" +echo " git.branch : $GIT_BRANCH" +echo "Intermediate profiler output kept at $INTERMEDIATE_DIR" From c5338f82654037db27511e2bccc8e4ce4282d182 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Tue, 4 Aug 2026 11:22:56 +0300 Subject: [PATCH 3/3] MS-1440 Add app startup benchmarking module --- benchmark/.gitignore | 1 + benchmark/build.gradle.kts | 42 +++++ benchmark/src/main/AndroidManifest.xml | 1 + .../testing/benchmark/StartupBenchmark.kt | 32 ++++ .../AndroidApplicationConventionPlugin.kt | 9 + .../metrics/compose-migration-metrics.md | 35 +++- .../results/fragments/startup_time.json | 9 + .../parse-startup-benchmark-results.py | 156 ++++++++++++++++++ .../scripts/run-startup-benchmark-metrics.sh | 28 ++++ gradle/libs.versions.toml | 3 + id/build.gradle.kts | 3 + id/src/benchmark/AndroidManifest.xml | 13 ++ settings.gradle.kts | 1 + 13 files changed, 325 insertions(+), 8 deletions(-) create mode 100644 benchmark/.gitignore create mode 100644 benchmark/build.gradle.kts create mode 100644 benchmark/src/main/AndroidManifest.xml create mode 100644 benchmark/src/main/java/com/simprints/testing/benchmark/StartupBenchmark.kt create mode 100644 docs/migration/metrics/results/fragments/startup_time.json create mode 100644 docs/migration/metrics/scripts/parse-startup-benchmark-results.py create mode 100644 docs/migration/metrics/scripts/run-startup-benchmark-metrics.sh create mode 100644 id/src/benchmark/AndroidManifest.xml diff --git a/benchmark/.gitignore b/benchmark/.gitignore new file mode 100644 index 0000000000..796b96d1c4 --- /dev/null +++ b/benchmark/.gitignore @@ -0,0 +1 @@ +/build diff --git a/benchmark/build.gradle.kts b/benchmark/build.gradle.kts new file mode 100644 index 0000000000..430578517b --- /dev/null +++ b/benchmark/build.gradle.kts @@ -0,0 +1,42 @@ +plugins { + alias(libs.plugins.android.test) +} + +android { + namespace = "com.simprints.testing.benchmark" + + compileSdk = 37 + defaultConfig { + minSdk = 31 + targetSdk = 37 + + testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" + testInstrumentationRunnerArguments["androidx.benchmark.suppressErrors"] = "EMULATOR" + } + + buildTypes { + // This benchmark buildType is used for benchmarking, and should function like your release build (for example, with minification on). + // It's signed with a debug key for easy local/CI testing. + create("benchmark") { + isDebuggable = true + signingConfig = getByName("debug").signingConfig + matchingFallbacks += listOf("release") + } + } + + targetProjectPath = ":id" + experimentalProperties["android.experimental.self-instrumenting"] = true +} + +dependencies { + implementation(libs.benchmark.macro.junit4) + implementation(libs.testing.androidX.ext.junit) + implementation(libs.testing.androidX.uiAutomator) + implementation(libs.testing.espresso.core) +} + +androidComponents { + beforeVariants(selector().all()) { + it.enable = it.buildType == "benchmark" + } +} diff --git a/benchmark/src/main/AndroidManifest.xml b/benchmark/src/main/AndroidManifest.xml new file mode 100644 index 0000000000..cc947c5679 --- /dev/null +++ b/benchmark/src/main/AndroidManifest.xml @@ -0,0 +1 @@ + diff --git a/benchmark/src/main/java/com/simprints/testing/benchmark/StartupBenchmark.kt b/benchmark/src/main/java/com/simprints/testing/benchmark/StartupBenchmark.kt new file mode 100644 index 0000000000..fafa534681 --- /dev/null +++ b/benchmark/src/main/java/com/simprints/testing/benchmark/StartupBenchmark.kt @@ -0,0 +1,32 @@ +package com.simprints.testing.benchmark + +import androidx.benchmark.macro.StartupMode +import androidx.benchmark.macro.StartupTimingMetric +import androidx.benchmark.macro.junit4.MacrobenchmarkRule +import androidx.test.ext.junit.runners.* +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Basic startup timing test + */ +@RunWith(AndroidJUnit4::class) +class StartupBenchmark { + @get:Rule + val benchmarkRule = MacrobenchmarkRule() + + @Test + fun startup() = benchmarkRule.measureRepeated( + packageName = "com.simprints.id", + metrics = listOf(StartupTimingMetric()), + iterations = 5, + startupMode = StartupMode.COLD, + setupBlock = { + killProcess() + pressHome() + }, + ) { + startActivityAndWait() + } +} diff --git a/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt index b8d5edbbdf..f8d920cac3 100644 --- a/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt @@ -106,6 +106,15 @@ class AndroidApplicationConventionPlugin : Plugin { } } } + + create("benchmark") { + initWith(buildTypes.getByName("release")) + signingConfig = signingConfigs.getByName("debug") + matchingFallbacks += listOf("release") + isMinifyEnabled = true + isShrinkResources = true + isDebuggable = false + } } } } diff --git a/docs/migration/metrics/compose-migration-metrics.md b/docs/migration/metrics/compose-migration-metrics.md index 50a5752093..8c2ead7928 100644 --- a/docs/migration/metrics/compose-migration-metrics.md +++ b/docs/migration/metrics/compose-migration-metrics.md @@ -16,13 +16,14 @@ This document defines a repeatable process to collect all migration metrics into ## 2. Metrics and where they come from -| Metric | Source | Automated by | -|--------------------------------|----------------------------------------------------------------------------|-------------------------------------------------------------------| -| Module Conversion % | `docs/migration/metrics/compose-module-status.csv` + `settings.gradle.kts` | `collect-compose-migration-metrics.py` | -| View-Compose Interop Nodes | Kotlin source scan (`ComposeView`, `AndroidView`) | `collect-compose-migration-metrics.py` | -| Code Ignored in Coverage | `@ExcludedFromGeneratedTestCoverageReports` usage + optional JaCoCo XML | `collect-compose-migration-metrics.py` | -| Build Time (clean/incremental) | `gradle-profiler` benchmark CSV | `run-gradle-profiler-metrics.sh` + `parse-gradle-profiler-csv.py` | -| APK Download Size | AAB artifact size | `record-metric-fragment.py` | +| Metric | Source | Automated by | +|--------------------------------|----------------------------------------------------------------------------|---------------------------------------------------------------------------| +| Module Conversion % | `docs/migration/metrics/compose-module-status.csv` + `settings.gradle.kts` | `collect-compose-migration-metrics.py` | +| View-Compose Interop Nodes | Kotlin source scan (`ComposeView`, `AndroidView`) | `collect-compose-migration-metrics.py` | +| Code Ignored in Coverage | `@ExcludedFromGeneratedTestCoverageReports` usage + optional JaCoCo XML | `collect-compose-migration-metrics.py` | +| Build Time (clean/incremental) | `gradle-profiler` benchmark CSV | `run-gradle-profiler-metrics.sh` + `parse-gradle-profiler-csv.py` | +| APK Download Size | AAB artifact size | `record-metric-fragment.py` | +| App Startup Time | `:benchmark:connectedBenchmarkAndroidTest` benchmark JSON output | `run-startup-benchmark-metrics.sh` + `parse-startup-benchmark-results.py` | ## 3. Step-by-step collection @@ -65,7 +66,19 @@ python3 docs/migration/metrics/scripts/record-metric-fragment.py \ apk_size.aab_bytes=$(stat -f%z id/build/outputs/bundle/debug/id-debug.aab) ``` -### Step 4 - Merge it into the unified snapshot: +### Step 4 - Collect app startup metrics + +Run startup benchmark tests from the benchmark module (requires a connected benchmark-capable device/emulator): + +```bash +bash docs/migration/metrics/scripts/run-startup-benchmark-metrics.sh +``` + +This writes a fragment: + +- `docs/migration/metrics/results/fragments/startup_time.json` + +### Step 5 - Merge it into the unified snapshot: ```bash python3 docs/migration/metrics/scripts/collect-compose-migration-metrics.py @@ -92,5 +105,11 @@ python3 docs/migration/metrics/scripts/collect-compose-migration-metrics.py - `docs/migration/metrics/scripts/parse-gradle-profiler-csv.py` Parses `benchmark.csv` and computes average scenario times in seconds. +- `docs/migration/metrics/scripts/run-startup-benchmark-metrics.sh` + Runs `:benchmark:connectedBenchmarkAndroidTest`, parses startup benchmark JSON output, and emits `startup_time.json`. + +- `docs/migration/metrics/scripts/parse-startup-benchmark-results.py` + Parses benchmark output JSON files and computes startup metric aggregates. + - `docs/migration/metrics/scripts/record-metric-fragment.py` Writes/updates a JSON fragment from `key=value` arguments for manual or scripted ingestion. diff --git a/docs/migration/metrics/results/fragments/startup_time.json b/docs/migration/metrics/results/fragments/startup_time.json new file mode 100644 index 0000000000..472bc3b21a --- /dev/null +++ b/docs/migration/metrics/results/fragments/startup_time.json @@ -0,0 +1,9 @@ +{ + "startup_time.git_branch": "spike/ms-1440-compose-migration", + "startup_time.git_commit": "b54f017c694b874dbc8292c49c00870c5bcc2d32", + "startup_time.timetoinitialdisplayms_avg": 633.4855, + "startup_time.timetoinitialdisplayms_max": 852.8743, + "startup_time.timetoinitialdisplayms_median": 586.0686, + "startup_time.timetoinitialdisplayms_min": 419.9393, + "startup_time.timetoinitialdisplayms_sample_count": 5.0 +} diff --git a/docs/migration/metrics/scripts/parse-startup-benchmark-results.py b/docs/migration/metrics/scripts/parse-startup-benchmark-results.py new file mode 100644 index 0000000000..ed9d6a241c --- /dev/null +++ b/docs/migration/metrics/scripts/parse-startup-benchmark-results.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +"""Parse benchmark module startup test results and emit startup metric fragment JSON.""" + +from __future__ import annotations + +import argparse +import json +import re +from collections import defaultdict +from pathlib import Path +from typing import Any + + +BENCHMARK_FILE_PATTERNS = ( + "**/*benchmarkData.json", + "**/*benchmarkData*.json", +) + + +def safe_key(raw: str) -> str: + return re.sub(r"[^a-z0-9]+", "_", raw.strip().lower()).strip("_") + + +def to_float(value: Any) -> float | None: + if isinstance(value, (int, float)): + return float(value) + return None + + +def extract_runs(metric_payload: Any) -> list[float]: + if isinstance(metric_payload, list): + return [v for value in metric_payload if (v := to_float(value)) is not None] + if not isinstance(metric_payload, dict): + return [] + + for key in ("runs", "samples", "values", "data"): + value = metric_payload.get(key) + if isinstance(value, list): + return [v for item in value if (v := to_float(item)) is not None] + return [] + + +def extract_mean(metric_payload: Any) -> float | None: + if not isinstance(metric_payload, dict): + return None + for key in ("mean", "average", "avg", "median", "p50"): + value = to_float(metric_payload.get(key)) + if value is not None: + return value + return None + + +def benchmark_entries(payload: Any) -> list[dict[str, Any]]: + if isinstance(payload, dict): + benchmarks = payload.get("benchmarks") + if isinstance(benchmarks, list): + return [item for item in benchmarks if isinstance(item, dict)] + if "metrics" in payload and isinstance(payload["metrics"], dict): + return [payload] + if isinstance(payload, list): + return [item for item in payload if isinstance(item, dict)] + return [] + + +def parse_file(path: Path) -> dict[str, list[float]]: + payload = json.loads(path.read_text(encoding="utf-8")) + by_metric: dict[str, list[float]] = defaultdict(list) + for benchmark in benchmark_entries(payload): + metrics = benchmark.get("metrics") + if not isinstance(metrics, dict): + continue + for raw_metric_name, metric_payload in metrics.items(): + metric_name = str(raw_metric_name).strip() + if not metric_name: + continue + runs = extract_runs(metric_payload) + if runs: + by_metric[metric_name].extend(runs) + continue + mean = extract_mean(metric_payload) + if mean is not None: + by_metric[metric_name].append(mean) + return by_metric + + +def parse_results(input_dir: Path) -> dict[str, float]: + files: list[Path] = [] + for pattern in BENCHMARK_FILE_PATTERNS: + files.extend(input_dir.glob(pattern)) + files = sorted(set(path.resolve() for path in files if path.is_file())) + if not files: + raise FileNotFoundError( + f"No benchmark JSON files found under {input_dir}. " + "Run :benchmark:connectedBenchmarkAndroidTest first." + ) + + by_metric: dict[str, list[float]] = defaultdict(list) + for file_path in files: + parsed = parse_file(file_path) + for metric_name, runs in parsed.items(): + by_metric[metric_name].extend(runs) + + startup_metric_names = [ + name + for name in by_metric.keys() + if "startup" in name.lower() or "display" in name.lower() + ] + + metric_names = startup_metric_names if startup_metric_names else list(by_metric.keys()) + + result: dict[str, float] = {} + for metric_name in sorted(metric_names): + runs = by_metric.get(metric_name, []) + if not runs: + continue + safe_metric = safe_key(metric_name) + sample_count = len(runs) + avg = sum(runs) / sample_count + sorted_runs = sorted(runs) + mid = sample_count // 2 + median = ( + sorted_runs[mid] + if sample_count % 2 == 1 + else (sorted_runs[mid - 1] + sorted_runs[mid]) / 2.0 + ) + result[f"startup_time.{safe_metric}_avg"] = round(avg, 4) + result[f"startup_time.{safe_metric}_median"] = round(median, 4) + result[f"startup_time.{safe_metric}_min"] = round(sorted_runs[0], 4) + result[f"startup_time.{safe_metric}_max"] = round(sorted_runs[-1], 4) + result[f"startup_time.{safe_metric}_sample_count"] = float(sample_count) + return result + + +def main() -> None: + parser = argparse.ArgumentParser() + parser.add_argument( + "--input-dir", + default="benchmark/build/outputs/connected_android_test_additional_output", + ) + parser.add_argument( + "--output-json", + default="docs/migration/metrics/results/fragments/startup_time.json", + ) + args = parser.parse_args() + + input_dir = Path(args.input_dir).resolve() + output_json = Path(args.output_json).resolve() + output_json.parent.mkdir(parents=True, exist_ok=True) + + metrics = parse_results(input_dir) + output_json.write_text(json.dumps(metrics, indent=2, sort_keys=True) + "\n", encoding="utf-8") + print(f"Wrote {output_json}") + + +if __name__ == "__main__": + main() diff --git a/docs/migration/metrics/scripts/run-startup-benchmark-metrics.sh b/docs/migration/metrics/scripts/run-startup-benchmark-metrics.sh new file mode 100644 index 0000000000..ed2f592b0a --- /dev/null +++ b/docs/migration/metrics/scripts/run-startup-benchmark-metrics.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash + +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "$0")/../../../.." && pwd)" +echo "Root: $ROOT_DIR" + +FRAGMENT_OUT="$ROOT_DIR/docs/migration/metrics/results/fragments/startup_time.json" +RESULTS_DIR="$ROOT_DIR/benchmark/build/outputs/connected_android_test_additional_output" + +GIT_COMMIT="$(git -C "$ROOT_DIR" rev-parse HEAD 2>/dev/null || echo "unknown")" +GIT_BRANCH="$(git -C "$ROOT_DIR" rev-parse --abbrev-ref HEAD 2>/dev/null || echo "unknown")" + +echo "Running startup benchmark for commit=$GIT_COMMIT branch=$GIT_BRANCH" +echo "Make sure a benchmark-capable device/emulator is connected." + +"$ROOT_DIR/gradlew" :benchmark:connectedBenchmarkAndroidTest + +python3 "$ROOT_DIR/docs/migration/metrics/scripts/parse-startup-benchmark-results.py" \ + --input-dir "$RESULTS_DIR" \ + --output-json "$FRAGMENT_OUT" + +python3 "$ROOT_DIR/docs/migration/metrics/scripts/record-metric-fragment.py" \ + "$FRAGMENT_OUT" \ + "startup_time.git_commit=$GIT_COMMIT" \ + "startup_time.git_branch=$GIT_BRANCH" + +echo "Startup metrics fragment written to $FRAGMENT_OUT" diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 0288512f22..334b3eafb7 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -85,6 +85,8 @@ protobuf_plugin_version = "0.10.0" deps_graph_version = "0.8.0" tink_version = "1.23.0" desugar_jdk_libsVersion = "2.1.5" +benchmark_macro_version = "1.4.1" + [libraries] #Kotlin @@ -248,6 +250,7 @@ testing-AndroidX-orchestrator = { module = "androidx.test:orchestrator", version testing-AndroidX-runner = { module = "androidx.test:runner", version.ref = "androidx_version" } testing-AndroidX-room = { module = "androidx.room:room-testing", version.ref = "androidx_room_version" } testing-AndroidX-uiAutomator = { module = "androidx.test.uiautomator:uiautomator", version.ref = "uiAutomator_version" } +benchmark-macro-junit4 = { group = "androidx.benchmark", name = "benchmark-macro-junit4", version.ref = "benchmark_macro_version" } # Dependencies of the included build-logic plugin-gradle-android = { group = "com.android.tools.build", name = "gradle", version.ref = "android_gradlePlugin_version" } diff --git a/id/build.gradle.kts b/id/build.gradle.kts index 33419bf3c4..b20c3ede6c 100644 --- a/id/build.gradle.kts +++ b/id/build.gradle.kts @@ -29,6 +29,9 @@ android { getByName("debug") { proguardFiles(getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro") } + getByName("benchmark") { + proguardFiles(getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro") + } } } diff --git a/id/src/benchmark/AndroidManifest.xml b/id/src/benchmark/AndroidManifest.xml new file mode 100644 index 0000000000..78be840f8f --- /dev/null +++ b/id/src/benchmark/AndroidManifest.xml @@ -0,0 +1,13 @@ + + + + + + + + + + diff --git a/settings.gradle.kts b/settings.gradle.kts index 2b2c5a8fa5..db1c14c7dd 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -155,4 +155,5 @@ include( // Test modules include( ":testing:data-generator", + ":benchmark", )