incubator/macros

Watch

Fork 0

Deepening pass: six improve-phase changes #33

Merged

morten-olsen merged 8 commits from refactor/deepening-pass into main

2026-09-17 13:14:10 +02:00

morten-olsen commented

2026-09-17 13:12:51 +02:00

Owner

Phase 7 of the package-architecture project — the improve loop the spec
hands off to. Six deepening changes, one per commit, fast-forwardable.

Each commit passes task ci:quality on its own.

What changed

#	Commit	Why
1	`refactor(engine)` delete the automation gateway pass-through	One-method interface over a two-line body, and `AutomationGraph.gateway` had no reader at all
2	`refactor(contract)` move the workflow codec into `:engine`	Failed the admission rule — no two consumers that must not depend on each other (decision 13)
3	`refactor(app)` construct the composition root once	Guardrail 4 said "constructed once"; 13 sites in `main` said otherwise
4	`refactor(engine)` lift the workflow editing algebra out of the view model	~150 lines of edge surgery behind `AndroidViewModel`, untested in either source set
5	`test(persistence)` hold both port adapters to one conformance suite	The shipped adapters had no tests; the gate reported green on an empty module
6	`refactor(platform)` declare each device reading once	Five declarations per reading across two modules, two already drifted

Plus a dead wildcard import that made the ui -> platform watch-item
tripwire unmeasurable, and the spec update recording all of it.

Numbers

	before	after
`sibling-imports.baseline`	19	14 — `events -> actions` now empty
tests in `:persistence`	0	54
JVM tests	~230	315
`:contract` main lines	1,097	794
`AutomationGraph` construction sites in `main`	13	1

Two drifts this fixes

Both were live on main:

The audio action declared a devices output, audioSnapshot emitted
it, and the audio event's hand-written payload schema denied it — so
the output picker offered the field on one path and not the other.
BuiltInEventRegistry called the power trigger "Power connected
changed" while BackgroundResourceManager called the same trigger
"Power connection changed", depending on which screen you were on.

Deliberate contract change

The audio event's payload schema gains the devices property it was
already carrying. Required sets are unchanged on both sides, so nothing
that validated before stops validating — the action's output schema is
now the looser event-contract one, which its own reading has always
satisfied in full.

New dependency, worth a look

Robolectric, test-only, on :persistence. It runs the real Android
SQLite on the JVM, which is what puts the adapters that actually ship
inside task ci:quality instead of behind an emulator. Apache-2.0, does
not ship, F-Droid unaffected — but it fetches an android-all jar from
Maven Central on first run, so CI needs that reachable. Straightforward
to back out in favour of instrumented runs if you would rather.

Verification

task ci:quality green after every commit.
./gradlew :app:connectedDebugAndroidTest — 68 tests, 0 failures
on macros_pixel9_api36. Required by the spec's risk register after a
class move, and this branch moves several.
Manual emulator smoke: cold launch, seeded demo workflow listed, ran it
to "Run completed", run persisted and shown in the Activity tab.

Follow-up filed as #32 — an automated smoke E2E so that last step stops
being hand-driven.

Phase 7 of the package-architecture project — the improve loop the spec hands off to. Six deepening changes, one per commit, fast-forwardable. Each commit passes `task ci:quality` on its own. ## What changed | # | Commit | Why | |---|---|---| | 1 | `refactor(engine)` delete the automation gateway pass-through | One-method interface over a two-line body, and `AutomationGraph.gateway` had no reader at all | | 2 | `refactor(contract)` move the workflow codec into `:engine` | Failed the admission rule — no two consumers that must not depend on each other (decision 13) | | 3 | `refactor(app)` construct the composition root once | Guardrail 4 said "constructed once"; 13 sites in `main` said otherwise | | 4 | `refactor(engine)` lift the workflow editing algebra out of the view model | ~150 lines of edge surgery behind `AndroidViewModel`, untested in either source set | | 5 | `test(persistence)` hold both port adapters to one conformance suite | The shipped adapters had no tests; the gate reported green on an empty module | | 6 | `refactor(platform)` declare each device reading once | Five declarations per reading across two modules, two already drifted | Plus a dead wildcard import that made the `ui -> platform` watch-item tripwire unmeasurable, and the spec update recording all of it. ## Numbers | | before | after | |---|---|---| | `sibling-imports.baseline` | 19 | 14 — `events -> actions` now empty | | tests in `:persistence` | 0 | 54 | | JVM tests | ~230 | 315 | | `:contract` main lines | 1,097 | 794 | | `AutomationGraph` construction sites in `main` | 13 | 1 | ## Two drifts this fixes Both were live on `main`: - The audio action declared a `devices` output, `audioSnapshot` emitted it, and the audio event's hand-written payload schema denied it — so the output picker offered the field on one path and not the other. - `BuiltInEventRegistry` called the power trigger "Power connected changed" while `BackgroundResourceManager` called the same trigger "Power connection changed", depending on which screen you were on. ## Deliberate contract change The audio event's payload schema gains the `devices` property it was already carrying. Required sets are unchanged on both sides, so nothing that validated before stops validating — the action's output schema is now the looser event-contract one, which its own reading has always satisfied in full. ## New dependency, worth a look Robolectric, test-only, on `:persistence`. It runs the real Android SQLite on the JVM, which is what puts the adapters that actually ship inside `task ci:quality` instead of behind an emulator. Apache-2.0, does not ship, F-Droid unaffected — but it fetches an `android-all` jar from Maven Central on first run, so CI needs that reachable. Straightforward to back out in favour of instrumented runs if you would rather. ## Verification - `task ci:quality` green after every commit. - `./gradlew :app:connectedDebugAndroidTest` — **68 tests, 0 failures** on `macros_pixel9_api36`. Required by the spec's risk register after a class move, and this branch moves several. - Manual emulator smoke: cold launch, seeded demo workflow listed, ran it to "Run completed", run persisted and shown in the Activity tab. Follow-up filed as #32 — an automated smoke E2E so that last step stops being hand-driven.

morten-olsen added 8 commits

2026-09-17 13:12:51 +02:00

refactor(engine): delete the automation gateway pass-through f91d6408f8

AutomationGateway was a one-method interface over a two-line body:
look the workflow up in the catalog, then run it. Its only construction
site exposed `AutomationGraph.gateway`, which nothing read — the whole
seam had no caller and no second adapter.

AutomationInvocation existed only to be that method's parameter object,
and it carried a CallerIdentity the implementation dropped on the floor,
so it goes with it and :contract sheds a type.

Renames the file after AutomationCatalog, which is what it still holds.

Left alone deliberately: InMemoryAutomationCatalog now has no consumer
at all, but removing it is a separate question from removing the gateway.

refactor(contract): move the workflow codec into :engine 456dd888b0

Admission to :contract requires two consumers that must not depend on
each other. AutomationJsonCodec has none: :platform never references it,
and its production consumers are :engine and :persistence — which may
depend on :engine by decision 5. It was admitted as a rider on
automation/model, which does qualify.

Moves it beside RunCheckpointCodec, the precedent already in the tree:
an :engine-side codec that encodes an :engine type and calls down into
this one. The model stays in :contract; only the serialisation leaves.

:contract main drops from 1,097 to 794 lines, well under the 1,200-line
watch item, and the codec stops being the one file in the module with no
comment naming its two independent consumers.

refactor(app): construct the composition root once 2353e3dd5f

Spec guardrail 4 says AutomationGraph "lives in :app, is constructed
once, and is never passed across a module boundary". Two of those three
held; "constructed once" did not. Thirteen sites in main built their own,
including the EventPublisher installed for every platform broadcast — so
each event reopened SQLite, rebuilt nine action providers and re-derived
38 descriptors before publishing.

The graph also hid the consequence from itself: backgroundResources and
workflowStartGuard were borrowed off MacrosApplication through an
`as? MacrosApplication` cast, falling back to fresh instances when the
cast missed. A fallback WorkflowStartGuard keeps its own circuit-breaker
state, so the safety mechanism could silently fork.

Moves the single instance onto MacrosApplication, reaches it through a
Context accessor, and makes the constructor internal. The cast is gone,
and with it the AutomationGraph -> MacrosApplication edge the spec calls
out in its dependency audit.

Components still receive the whole graph; narrowing them to the services
they actually use is the next step, not this one.

refactor(engine): lift the workflow editing algebra out of the view model b7b09a7d8d

Edge splicing, bypass on delete, chain reordering and trigger root
reconnection were five methods on MacrosViewModel — the most intricate
logic in :app, reachable only through AndroidViewModel, and with no test
of any kind in either source set.

They were already pure: Automation in, Automation out, over :contract and
:engine types only. Moves them to :engine as a total editing algebra —
an edit the graph cannot accept returns the workflow unchanged rather
than throwing or half-applying — and brings the shape-snapshot helper
with them, since resolving a node's ports is engine work either way.

Node identity is now supplied rather than minted inside, which is what
makes them pure. The view model keeps the UUID and the state lookups.

Adds 20 JVM tests covering the cases that had none: splice reconnection,
fan-out refusal, branch-port edges dropped rather than bypassed onto the
wrong successor, switch-with-branches refusal, trigger root adoption,
duplicate-source refusal, and the last-trigger guard.

MacrosViewModel drops from 819 to 690 lines and sheds one of its eleven
responsibilities.

test(persistence): hold both port adapters to one conformance suite 26692d1fba

Every engine port has two adapters — an in-memory one in :engine and the
SQLite/SharedPreferences one that actually ships. Only the in-memory twin
was tested. :persistence had no test source directory at all, so
`task ci:quality` ran :persistence:testDebugUnitTest against nothing and
reported green; the shipped adapters were covered only by one 178-line
device test outside the gate.

Adds a conformance suite per port, run against both adapters:

  RunJournalContractTest   14 tests x 2 adapters
  EventStoreContractTest   13 tests x 2 adapters

The lease rule is the reason this matters. "Claim once, and again only
once the lease expires" is written twice — as Kotlin in InMemoryRunJournal
and as SQL in SqliteRunJournal — and nothing asserted they agreed. Same
for inbox deduplication and continuation correlation, where a miss either
runs a workflow twice or wakes the wrong suspended run. Both adapters pass
all 54.

Adds Robolectric as a test-only dependency so the real Android SQLite runs
on the JVM and the shipped adapters land inside the gate rather than
behind an emulator. It does not ship, does not reach consumers, and is
Apache-2.0, so F-Droid compatibility is unaffected. It does fetch an
android-all jar from Maven Central on first run, which CI will need.

refactor(platform): declare each device reading once aa967bcd9e

A device reading — audio route, network, screen, power, Do Not Disturb —
was declared in five places: the snapshot function in `actions`, the
action's output schema, the event's payload schema hand-written in
`events` in a different notation, the receiver that built the payload,
and the intent-filter wiring. Nothing held them in agreement, and two had
already drifted:

  - `devices` was declared by the audio action and emitted by the audio
    snapshot, but absent from the audio event's payload schema, so the
    output picker offered the field on one path and not the other.
  - AudioRouteReceiver re-built five of the six audio fields by hand.

Moves the readings into the root platform package — which both `actions`
and `events` may import, and neither sibling may import the other — with
the fields declared once and the action and event schemas derived from
that declaration. The intent-supplied variants stay explicit rather than
being optional override parameters hanging off a snapshot function.

The five `events -> actions` entries leave sibling-imports.baseline,
taking it from 19 to 14 and clearing that direction entirely; `events`
now only reaches `scheduling` and `audio`.

Contract change, deliberate: the audio event's payload schema gains the
`devices` property it was always carrying. Required sets are unchanged on
both sides, so nothing that validated before stops validating — the
action's output schema is now the looser event-contract one, which its
own reading has always satisfied in full.

Adds five JVM tests asserting each event uses its reading's schema rather
than a copy, so a hand-written payload schema cannot creep back.

style(ui): drop a dead wildcard import of the platform root package aa313152f4

The root cloud.olsen.macros.platform package holds only FileReferenceAccess
and PlatformEventPublishing, and ScheduleSettingsScreen uses neither.

It also made the `ui -> platform` tripwire in the spec's watch items
unmeasurable: a wildcard admits new symbols without a diff showing them,
so counting named imports would silently undercount.

docs(architecture): record the first improve pass

ci/woodpecker/pr/compliance Pipeline was successful

Details

ci/woodpecker/pr/quality Pipeline failed

Details

ci/woodpecker/push/compliance Pipeline was successful

Details

ci/woodpecker/push/quality Pipeline was successful

Details

ci/woodpecker/push/draft-release Pipeline was successful

Details

ed04ebb601

Phase 7 is no longer backlog. Records what the six deepening changes
moved, so the spec and the code do not silently disagree:

- decision 13: AutomationJsonCodec refused admission to :contract after
  the fact, and why. The module catalog follows.
- the root platform package now carries DeviceReadings, and the baseline
  history says why it went from 19 to 14.
- guardrail 4 describes how "constructed once" is now enforced rather
  than merely asserted.
- watch items corrected: :contract is 794 lines, not ~920; the
  ui -> platform tail is 9 symbols, not 24.
- a new risk row for the gate reporting green on a module with no tests.

morten-olsen merged commit ed04ebb601 into main

2026-09-17 13:14:10 +02:00

morten-olsen deleted branch refactor/deepening-pass

2026-09-17 13:14:10 +02:00