From a1f4f51f08515bd7e3ab06dd9b70e7500da3921f Mon Sep 17 00:00:00 2001 From: Scott Olcott Date: Wed, 30 Sep 2026 11:29:32 -0600 Subject: [PATCH] Move area-specific guidance from CLAUDE.md into path-scoped rules CLAUDE.md loaded all 347 lines every session, though most of it only applies in one area. Split it into seven .claude/rules/ files that load when matching files are touched: gradle-build, hot-reload, data-flow, compose-stability, circuit, network and storage. CLAUDE.md keeps what is true everywhere and drops to 96 lines. The circuit-screen and themealdb-api skills move their invariants into the circuit and network rules, so the rules apply without the skill being invoked. The skills keep the add-a-screen walkthrough and the endpoint/tier reference. verify stays a skill. Agents: circuit-scaffold reads the circuit and data-flow rules and now requires @Immutable on the sealed state; recipes-gradle-runner learns composeStabilityCheck, which /verify delegates to it. Co-Authored-By: Claude Opus 5.5 --- .claude/agents/circuit-scaffold.md | 17 +- .claude/agents/recipes-gradle-runner.md | 5 +- .claude/rules/circuit.md | 79 +++++++ .claude/rules/compose-stability.md | 79 +++++++ .claude/rules/data-flow.md | 63 ++++++ .claude/rules/gradle-build.md | 50 +++++ .claude/rules/hot-reload.md | 58 +++++ .claude/rules/network.md | 83 +++++++ .claude/rules/storage.md | 19 ++ .claude/skills/circuit-screen/SKILL.md | 45 +--- .claude/skills/themealdb-api/SKILL.md | 72 +------ .claude/skills/verify/SKILL.md | 2 +- CLAUDE.md | 274 ++---------------------- 13 files changed, 475 insertions(+), 371 deletions(-) create mode 100644 .claude/rules/circuit.md create mode 100644 .claude/rules/compose-stability.md create mode 100644 .claude/rules/data-flow.md create mode 100644 .claude/rules/gradle-build.md create mode 100644 .claude/rules/hot-reload.md create mode 100644 .claude/rules/network.md create mode 100644 .claude/rules/storage.md diff --git a/.claude/agents/circuit-scaffold.md b/.claude/agents/circuit-scaffold.md index 493db06..12205e3 100644 --- a/.claude/agents/circuit-scaffold.md +++ b/.claude/agents/circuit-scaffold.md @@ -12,15 +12,17 @@ not a finished feature. ## Before writing anything -Read these two files, in this order. They are the contract: +Read these files, in this order. They are the contract: -1. `.claude/skills/circuit-screen/SKILL.md` — the pattern, the required declaration order, and the - traps. -2. `domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/CategoriesPresenter.kt` — +1. `.claude/skills/circuit-screen/SKILL.md` — the pattern and the required declaration order. +2. `.claude/rules/circuit.md` — the traps: `@Redacted` sinks, per-state events, + `@CircuitSerializable`, and URL mapping. +3. `domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/CategoriesPresenter.kt` — the canonical example. Match its structure, import style, and naming exactly. -If the screen reads a repository, also grep the repository interface in `repository/` and confirm -the method you intend to call actually exists. **Do not invent repository methods.** If nothing +If the screen reads a repository, also read `.claude/rules/data-flow.md` (`ContentState`, +`foldToState`, and why `hasAnswer` beats `data.isEmpty()`), and grep the repository interface in +`repository/` to confirm the method you intend to call actually exists. **Do not invent repository methods.** If nothing suitable exists, leave a TODO naming what's needed and carry on. ## What you produce @@ -34,6 +36,9 @@ Given a screen name `Xxx` and a description of what it shows: last (`com.slack.circuit.serialization.CircuitSerializable` — it implies `@Serializable`, so don't add that too; `Screen`s are not `Parcelable`). - `@CircuitInject(XxxScreen::class, AppScope::class)` and `@Inject` on the presenter + - `@Immutable` on the sealed `XxxState` (`@Stable` instead if a case holds an observable holder + such as a `TextFieldState`) — `composeStabilityCheck` fails an unannotated one; see + `.claude/rules/compose-stability.md` - `@Redacted` on every `eventSink` property - `retain { }` for retained state, never `rememberSaveable` - `ContentState.foldToState(onLoading, onError, onContent)` to map producer output onto the diff --git a/.claude/agents/recipes-gradle-runner.md b/.claude/agents/recipes-gradle-runner.md index 0286f74..3de2228 100644 --- a/.claude/agents/recipes-gradle-runner.md +++ b/.claude/agents/recipes-gradle-runner.md @@ -33,9 +33,12 @@ check, don't guess. | Check formatting without writing | `ktfmtCheck` | | Static analysis | `detektAll` | | Sort build-file dependency blocks | `sortDependencies` / `checkSortDependencies` | +| Compose stability (`:ui :domain :shared :app :webApp :desktopApp`) | `composeStabilityCheck` | | Everything | `build` | -`detektAll` is a custom aggregate task, not stock detekt. `::build` is the project's +`detektAll` is a custom aggregate task, not stock detekt. Run `composeStabilityCheck` as its own +invocation, never combined with `build` in one command: every target writes the same reports +directory, so its result is only trustworthy on its own. On failure, relay the offenders it names. `::build` is the project's standard compile check — prefer it over `assemble` unless the caller asked for something narrower. Run from the repo root with `./gradlew`. Add `--console=plain`; it strips the progress-bar control diff --git a/.claude/rules/circuit.md b/.claude/rules/circuit.md new file mode 100644 index 0000000..bdd21e4 --- /dev/null +++ b/.claude/rules/circuit.md @@ -0,0 +1,79 @@ +--- +paths: + - "domain/**" + - "ui/**" +--- + +# Circuit screens, state, and navigation + +A presenter (`:domain`) and its UI (`:ui`) are joined only by +`@CircuitInject(XxxScreen::class, AppScope::class)` on each side. `:domain` never imports from +`:ui`. For the step-by-step of adding a screen, use `/circuit-screen`. + +## Presenter file layout + +`domain/.../presenter/XxxPresenter.kt`, in this order: presenter class → sealed +`XxxState : CircuitUiState` (Loading/Error/Success) → sealed `XxxEvent` with nested per-state +sub-interfaces → the `Screen` last, as `@CircuitSerializable(AppScope::class) data object XxxScreen : Screen`. + +- **`@Redacted` on every `eventSink`.** Comes from `dev.zacsweers.redacted.annotations.Redacted`; + keeps lambdas out of `toString`. +- **Events are nested per state**, not one flat sealed interface. A `Success` state's sink only + accepts `XxxEvent.Success`. +- A retry is conventionally an `Int` counter (`retryTrigger`) passed into the producer as a key, + incremented by `XxxEvent.Error.RetryClicked`. + +## Retained state + +State survives recomposition/config change via `androidx.compose.runtime.retain.retain { }`, +**not** `rememberSaveable`. One sanctioned exception: `HomePresenter` persists the selected tab +with `rememberSerializable`, because that selection is cheap to store and worth surviving process +death so the app reopens on the tab the user left. Don't "fix" it to `retain { }`. + +**`:domain` depends on Material3 (`api`), deliberately.** `SearchState` holds a `SearchBarState` +and `RecipeScaffoldState`/`SearchScreen` carry a `SearchBarValue`, so that the presenter — not the +composable — owns whether the search bar is expanded. This is the *only* sanctioned place for +Material3 widget state in `:domain`; other widget state belongs in `:ui`. + +## URL mapping + +A new screen must also be added to `domain/.../navigation/ScreenUrlMapper.kt` — both the +`Screen.toUrlPath()` and `urlPathToScreen()` directions. It backs `recipes://app/...` deep links +and browser history on web. Home tabs are the exception to writing anything there by hand: each +`HomeTabScreen` carries its own `urlSegment` and the mapper reads `HOME_TABS`, so adding a tab to +that one list makes it addressable in both directions. + +## Navigation persistence + +Since Circuit 0.38, `Screen` and `PopResult` are no longer `Parcelable`; the back stack is +persisted with kotlinx-serialization. + +- **`@CircuitSerializable(AppScope::class)` on the `Screen`**, from + `com.slack.circuit.serialization`. It is `@MetaSerializable`, so it already implies + `@Serializable` — don't add that too. +- Screens with parameters are a sealed interface whose **concrete cases** carry the annotation — + see `RecipesScreen.ByCategory` / `.ByArea` / `.BySearch` / `.Favorites`. The sealed parent is + never annotated. +- Every parameter of a screen must be serializable. `RecipeId`, `CategoryId` and `IngredientId` + already are; a new parameter type in `:model` needs `@Serializable` added there. +- `SubScreen`s from `circuitx-subcircuit` never enter the back stack and get **no** annotation — + they may hold unserializable state such as a `TextFieldState`. + +Circuit codegen turns each `@CircuitSerializable` screen into a `CircuitSerializerRegistration` +multibinding, and `domain/.../circuit/CircuitProviders.kt` folds the whole set into a +`SerializableCircuitSaver` on `Circuit.Builder`. Nothing to register by hand — but the failure +modes are runtime, not compile time: + +- The saver is installed with the static `setCircuitSaver(saver)` overload rather than the + `setCircuitSaver { fallback -> ... }` transform, so **saving** an unregistered or unserializable + screen **throws** instead of quietly falling through to the registry-backed saver. That is + deliberate. +- **Restoring** one can only return null, and the nav stack drops that record; `CircuitProviders` + passes `onRestoreError` to the logger so it isn't silent. + +**The nav stack is `rememberSaveableNavStack`, not `rememberSaveableBackStack`.** `BackStack` +stubs `forward()` and `backward()` to `false`, and `NavigatorImpl` delegates straight to them, so +switching to it silently turns the browser back/forward buttons on web into no-ops — +`ui/src/webMain/.../BrowserHistoryEffect.web.kt` drives navigation through +`Navigator.backward()`/`forward()`. Both `RecipeApp.kt` and `RecipeScaffoldPresenter.kt` must stay +on `rememberSaveableNavStack`. diff --git a/.claude/rules/compose-stability.md b/.claude/rules/compose-stability.md new file mode 100644 index 0000000..37edaf8 --- /dev/null +++ b/.claude/rules/compose-stability.md @@ -0,0 +1,79 @@ +--- +paths: + - "ui/**" + - "domain/**" + - "model/**" + - "repository/**" + - "shared/**" + - "app/**" + - "desktopApp/**" + - "webApp/**" + - "config/compose/**" +--- + +# Compose stability + +Everything a presenter hands to a composable has to be *stable*, or Compose re-runs the UI on every +recomposition. Two rules keep it that way: + +- **Collections headed for the UI are `ImmutableList`**, from the repository's public signature + onward — `SourceOfTruth.reader`, `toModel()`, the `Flow>` return type, the producer's + `ContentState>`, and the screen state's field. Entities and DTOs stay `List`: + Store's local type is `List` and nothing below `toModel()` ever reaches composition, so + converting there is a copy that buys nothing. `:model` carries + `api(libs.kotlinx.collections.immutable)` for this. +- **`@Immutable` when it is true, `@Stable` when it is not.** `@Immutable` promises no public + property ever changes: right for the `:model` classes and for a screen state whose fields are all + values (`AreasState`, `RecipesState`, `RecipeDetailsState`, `SearchTabState`, `RecipesScreen`). + A state that carries an observable holder — `SearchBarState` and `TextFieldState` in `SearchState`, + `NavStack`/`Navigator` in `RecipeScaffoldState` and `HomeState`, the Metro graph in + `DesktopAppGraph` — gets `@Stable`. Both make the type skippable; only one of them is honest, and + a class marked `@Stable` must back its mutable properties with snapshot state (this is why + `BackShortcutHost.onBack` is a `MutableState`). + +Annotate the *declared parameter type*: `@Stable` on a supertype does not propagate, so +`DesktopAppGraph` carries its own annotation rather than inheriting one from `AppGraph`. + +A sealed `CircuitUiState` interface is unstable until annotated — the compiler cannot see the +implementations — so every one of them carries a marker. + +## `composeStabilityCheck` + +**None of the above is taken on trust — `./gradlew composeStabilityCheck` enforces it.** The +`compose.stability` convention plugin turns on the Compose compiler's own metrics and reports for +the six modules that apply the compiler plugin (`:ui`, `:domain`, `:shared`, `:app`, `:webApp`, +`:desktopApp`) and registers a per-project check that reads them. It fails on a class the compiler +inferred as `unstable`, and on a `restartable` composable that is not `skippable`. A `runtime` +class passes: its stability depends on a generic argument and is settled at run time. There is no +aggregate task — Gradle's cross-project name matching fans `composeStabilityCheck` out to all six. + +Exceptions live in `config/compose/unstable-classes.txt` and +`config/compose/unskippable-composables.txt`, one `:` per line, each +with a comment saying why. Today only the Metro graph impls and the two Android entry points are +listed, none of which is ever a composable parameter. + +Reports land in `/build/reports/compose/reports/` (`-classes.txt`, `-composables.txt`) with +`-module.json` counts under `.../metrics//main/`. Read them for *why* something failed — +the entry lists each property or parameter and its verdict. The compiler names them after the +Kotlin module, which is the Gradle project path — so they carry a colon, `Recipes:domain-classes.txt`, +and CI stages a renamed copy before uploading them because `upload-artifact` rejects one. + +Two traps in those reports: + +- **Only the metrics path is per-target; the reports path is not.** The Compose plugin appends + `/` to `metricsDestination` but hands `reportsDestination` to every + compilation verbatim, and a KMP module's targets share a Kotlin module name. So all six targets + write the same `Recipes_ui-classes.txt` and the last to run wins. After a full `./gradlew build` + you are reading whichever target finished last. `composeStabilityCheck` is the invocation to + trust: it depends on exactly one compile task (JVM where there is one, `compileDebugKotlin` for + `:app`, `compileKotlinJs` for `:webApp`), so it has a single writer. That is also why it is not + wired into `check`, and why CI runs it as its own step *before* `build`. +- **`reportsDestination` is a compiler *input*, not an output.** A compile task whose reports have + been deleted still counts as up to date and quietly writes nothing. `compose.stability` declares + the directory as an output of that one designated compile task to fix it, which also means the + task re-runs after a full `build` and stays the last writer. + +Detekt's `UnstableCollections` covers the same ground at the source level and is on. It is right +about composables that emit UI and wrong about ones that return a value — those are neither +restartable nor skippable, so a parameter's stability is inert. `RecipesProducer.produceByIngredients` +is the single `@Suppress` for that reason. diff --git a/.claude/rules/data-flow.md b/.claude/rules/data-flow.md new file mode 100644 index 0000000..b036f23 --- /dev/null +++ b/.claude/rules/data-flow.md @@ -0,0 +1,63 @@ +--- +paths: + - "repository/**" + - "domain/**" +--- + +# Reading data: `Outcome` and `ContentState` + +Repositories return `Flow>`, not Store's own `StoreReadResponse`. **Store 5 is an +implementation detail of `:repository`** — it is an `implementation` dependency there, and nothing +above that module compiles against it. `io.github.solcott:dataresult-store5` does the translation +in `asOutcomes(fetching = refresh) { … }`, at the end of each repository's chain. It sits *inside* +the `flatMapLatest { refresh -> … }` so it can see whether a fetch is coming. + +The types come from [kmp-dataresult](https://github.com/solcott/kmp-dataresult), shared with the +`Countries` project. Resolving them needs `gpr.user`/`gpr.key` in `~/.gradle/gradle.properties` (a +classic PAT with `read:packages`) — GitHub Packages authenticates even public reads. + +The path from a repository to a screen state is three steps: + +1. `Flow>` — `Loading`, `Data(value, origin)` or `Error(cause, origin)`. Store's + `Initial` and `Loading` both become `Outcome.Loading`; `NoNewData` is dropped. An empty first + read from cache while a fetch is pending is held back until the fetch answers. Room returns `[]` + for a key it has never fetched, and that's a cache miss, not an empty result. + `getFavoritesAsFlow` passes no `fetching`, because nothing is coming to answer instead. +2. `produceRetainedContentState(initial, keys) { … }` from `libs.uistateCircuit` + (`io.github.solcott.uistate.circuit`) folds those emissions into a retained `ContentState`. + **`ContentState.data` is what holds the last loaded value** — the separate retained + `lastItems`/`lastRecipes` caches that used to sit beside the response are gone, and so is + `ListUi`. Shared with `Countries`; the local copy in `:domain`'s `producer` package is gone. +3. `ContentState.foldToState(onLoading, onError, onContent)` in `presenter/ListContent.kt` maps it + onto a screen's own three-case `CircuitUiState`. + +A screen fed by several sources at once uses the group versions of the first two steps. The +repository returns `combineOutcomes(a, b, c)`, a `Flow>` with each source seeded +with `Loading` so the fastest one shows at once. The presenter folds it with +`produceRetainedContentStates(contentStatesOf(…))` into one `ContentState` per source. Destructure +the group for names; `isAnyLoading`/`errorOrNull` on it answer for all of them. `SearchPresenter` is +the example. + +Two things are easy to get wrong: + +- **Check `hasAnswer`, never `data.isEmpty()`**, to tell "nothing has loaded yet" from "loaded and + empty". `hasLoaded` reads `origin`, which stays null until the first value arrives. `hasAnswer` + also refuses an empty read *from cache* while its request is in flight or has failed. That's a key + the database has never seen, not an empty result. The repositories already hold that read back + with `asOutcomes(fetching = refresh) { … }`, and `hasAnswer` is the backstop. Testing the data for + emptiness leaves a spinner over a legitimately empty tab forever. `hasAnswer` can't, because it is + only false while a request is outstanding or has failed. +- **Having data beats being in flight.** `foldToState` checks `hasAnswer` first, which is what + renders a background refresh as `isRefreshing` over the existing grid instead of dropping back to + a full-screen spinner. `domain`'s `refreshKeepsPreviousAreas` test pins this — don't reorder those + branches. + +`DataError` is structured; screen states carry `String`. `DataError.toMessage()` in +`ListContent.kt` bridges the two, and belongs in `:ui` with the rest of the user-facing copy once +these get localized. + +**A test fake for an in-flight request must not complete.** `produceRetainedContentState` settles a still +loading status when its source completes, so a spinner can never hang — which means +`flowOf(Outcome.Loading)` is a *settled empty result*, not a pending one. Store streams never +complete; use the `inFlight()` helper in the presenter tests, which emits `Loading` and then awaits +cancellation. diff --git a/.claude/rules/gradle-build.md b/.claude/rules/gradle-build.md new file mode 100644 index 0000000..e71f6e5 --- /dev/null +++ b/.claude/rules/gradle-build.md @@ -0,0 +1,50 @@ +--- +paths: + - "**/*.gradle.kts" + - "build-logic/**" + - "gradle/**" + - "gradle.properties" +--- + +# Gradle build and conventions + +## JDK 25, bytecode 17 + +Everything runs on **JDK 25** and everything compiles to **JVM 17** — those are two different +settings and the split is deliberate. The daemon uses Amazon Corretto 25, auto-provisioned via +foojay from `gradle/gradle-daemon-jvm.properties`; `jvm-toolchain = "25"` in the catalog is the +JDK that compiles the code and runs it (`:desktopApp:run`, tests, the jpackage runtime image), while +`jvmSourceCompatibility`/`jvmTargetCompatibility = "17"` is the bytecode and API level. +Don't override the JDK. + +A toolchain silently sets `jvmTarget` to its own version unless a target pins one, so **every JVM +compilation pins it explicitly** — `jvm {}` and `android {}` in `kmp.library`, plus `:app` and +`:desktopApp` — each with `jvmTarget` *and* `-Xjdk-release=17`, which additionally hides post-17 JDK +APIs so nothing that would crash on Android can compile. Drop either and you silently get bytecode +25. `-Xjdk-release` is JVM-only: never add it to the project-wide `compilerOptions`, which also +feeds Native, JS, and WasmJS. `kotlin.jvm.target.validation.mode` is unset (default `error`), so a +Kotlin/Java target mismatch fails the build rather than shipping — that check is a feature, not an +obstacle. + +## Module conventions + +- New library modules apply the **`kmp.library`** convention plugin + (`build-logic/src/main/kotlin/kmp.library.gradle.kts`), not raw KMP plugins. It sets the targets, + toolchain, Android namespace (`com.scottolcott.recipe.`), ktfmt, detekt, and sort-dependencies. +- **No `iosX64`** — Intel Macs can't run the iOS simulator build. +- `-Xexpect-actual-classes` is set project-wide by `kmp.library`. Don't re-declare it per file. + The `-opt-in=kotlin.time.ExperimentalTime` that used to sit beside it is gone: `kotlin.time.Clock` + and `Instant` went Stable in Kotlin 2.3.0, so neither the flag nor a per-file `@OptIn` is needed. +- `@Parcelize` comes from `io.github.solcott.kmp.parcelize` (a multiplatform plugin), **not** + kotlin-parcelize. It applies only to the `:model` id value classes (`RecipeId`, `CategoryId`, + `IngredientId`) — **`Screen`s are not Parcelable** (see `.claude/rules/circuit.md`). +- Any module declaring `Screen`s needs the kotlinx-serialization Gradle plugin + (`libs.plugins.kotlinx.serialization`); `:domain` already applies it. + +## Tooling tasks + +- `detektAll` is a custom aggregate task from `build-logic/src/main/kotlin/detekt.gradle.kts`. + Plain `detekt` does not cover all source sets. +- `buildHealth` (dependency-analysis) is noisy about Compose artifacts. Most modules already carry + `dependencyAnalysis { issues { onUnusedDependencies { exclude(...) } } }` — add to those rather + than deleting a dependency it flags. diff --git a/.claude/rules/hot-reload.md b/.claude/rules/hot-reload.md new file mode 100644 index 0000000..97edf57 --- /dev/null +++ b/.claude/rules/hot-reload.md @@ -0,0 +1,58 @@ +--- +paths: + - "desktopApp/**" + - "ui/**" +--- + +# Compose Hot Reload + +Swaps changed classes into the running desktop app instead of restarting it. **Nothing in the build +declares it** — the Compose Multiplatform Gradle plugin (1.12.0) auto-applies +`org.jetbrains.compose.hot-reload` (1.2.0) to every project with the Kotlin JVM or Multiplatform +plugin, so there is no alias in `libs.versions.toml` to bump and nothing to look for in +`desktopApp/build.gradle.kts`. `-Porg.jetbrains.compose.hot.reload.disable=true` turns it off. +The `exclude("org.jetbrains.compose.hot-reload:hot-reload-runtime-api")` lines in `:desktopApp`, +`:ui`, and `:domain` are dependency-analysis reacting to the artifact the plugin injects. + +`gradle.properties` sets `compose.reload.jbr.autoProvisioningEnabled=true`: hot reload needs a +JetBrains Runtime for enhanced class redefinition, and no JBR is installed system-wide, so Gradle +fetches one through the foojay resolver on first use (~200MB, once). It follows the project +toolchain, so it now provisions **JBR 25** (`jbrsdk_jcef-25.0.4.1`) and the daemon, the compile +toolchain, and the hot-reload JVM are all 25. An older `jbrsdk_jcef-21` may still be sitting in +`~/.gradle/jdks` from before the toolchain moved; it is unused. + +``` +./gradlew :desktopApp:hotRun --auto # continuous build; reloads on save +./gradlew :desktopApp:hotRun # explicit mode; reload with ./gradlew :desktopApp:reload +``` + +`:desktopApp` is a plain `kotlin("jvm")` module, so its tasks have **no target suffix** — +`hotRun`, `hotRunAsync`, `hotMcpServer`, `reload`. The KMP modules that apply Compose (`:ui`, +`:domain`, `:shared`) get their own `hotRunJvm`/`hotMcpServerJvm`; you never want those, and it is +why `.mcp.json` names `:desktopApp:hotMcpServer` in full rather than the bare `hotMcpServer` the +upstream README suggests — the short form matches every one of those projects at once. + +Editing a composable in `:ui` reloads into the `:desktopApp` window; cross-module reload works +because both are in the same build. + +**Reload output does not appear in the `hotRun` terminal.** `--auto` forks a second Gradle daemon for +the continuous build, and its recompile/reload log goes to `desktopApp/build/run/main/main.chr.log` — +read that to see "Change detected", the build result, and which `@Composable` scopes were invalidated. +The staged classes land in `desktopApp/build/run/main/classpath/hot/`. The `hotRun` terminal only +carries the app's own stdout. If a reload seems not to land, check that log before assuming the app +is wedged, and check for a stale app process from an earlier session — each `hotRun` starts its own. + +**MCP server.** `.mcp.json` registers `compose-hot-reload`, which exposes `status`, `reload`, +`await_reload`, `take_screenshot`, `get_semantic_tree`, `get_ui_error`, `get_logs`, `list_windows`, +`click`, `type_text`, `scroll`, `scroll_to_index`, `resize_window`, `restart`, and `reset_ui`. +The agent starts the server itself; it waits for the app and connects when it appears, so launch +`hotRun` separately. Check `status.buildContinuous` to pick between `reload` (explicit mode) and +`await_reload` (started with `--auto`). + +Two limits worth knowing here: + +- **The Metro graph is not reloaded.** `createGraph()` runs outside the composition + in `main.kt`, so DI and provider changes need `restart`, not `reload`. +- **Retained state survives a reload.** `retain { }` and presenter state persist across a swap — + usually what you want, but a state-shape change can leave stale values behind. `reset_ui` discards + the composition; `restart` restarts the process. diff --git a/.claude/rules/network.md b/.claude/rules/network.md new file mode 100644 index 0000000..0204a76 --- /dev/null +++ b/.claude/rules/network.md @@ -0,0 +1,83 @@ +--- +paths: + - "network/**" + - "repository/**" +--- + +# TheMealDB network layer + +Endpoint table, tier limits, image URLs, and attribution: `/themealdb-api`. + +## Base URL and key + +This app uses TheMealDB **v2 exclusively**; there is no v1 fallback. + +``` +https://www.themealdb.com/api/json/v2/{MEALDB_API_KEY}/ +``` + +Built in exactly one place: `network/.../NetworkProviders.kt` (`MEALDB_BASE_URL` + `DefaultRequest`). +Every `@Resource` path is relative to that trailing slash, so no resource declares a version. + +`MEALDB_API_KEY` is **required**. It flows Gradle property or env var → +`build-logic/.../ProjectExt.kt` → BuildKonfig `SharedBuildConfig.MEALDB_API_KEY` (`shared/build.gradle.kts`) +→ each platform's `RuntimeConfigImpl` → `RuntimeConfig.mealDbApiKey: String` (non-null). A missing or +blank key fails the Gradle build with a message pointing at the signup page. + +**The key is in the URL path, not a header.** No `LogLevel` short of `NONE` omits the request URL, so +`NetworkProviders.redacting()` wraps the Ktor logger and rewrites the key to `***`. Preserve that +wrapper if you touch the logging block, and never add a second client that logs API URLs unredacted. + +**Why v2 only:** with a real key, v2 returns everything v1 does plus more results and four extra +endpoints. But **v2 with the free dev key silently caps `filter.php` at one result**, which would +reduce every browse-by-category screen to a single recipe and look like an app bug rather than a +tier limit. Hence: one base URL, and a build that fails without a key rather than shipping that. +If a filter suddenly returns exactly one meal, suspect the key before suspecting the code. + +Two `Json` qualifiers exist, `@NetworkJson` and `@StorageJson` +(`core/.../serialization/JsonQualifiers.kt`). Pick deliberately. + +## Resources + +New resources mirror an existing sibling in `network/.../resource/` (all are `internal class`, not +`data class`) and its `api/` consumer. Spaces in `c` / `a` / `i` values may be written as +underscores (`chicken_breast`). URL-encode anything user-supplied. + +**`list.php?a=list` and `filter.php?a=` do not agree on what an area is.** The list returns ~195 +rows with a real key, but `filter.php` matches the exact `strArea` string meals actually carry — +14 distinct values on v1. Most rows in the area list therefore have no recipes behind them. +`RecipeApiImpl.getByArea` asks for both spellings the list gives (`Italian` and `Italy`) and merges +the results, which recovers some but not all of the gap. If the Areas tab looks full of dead ends, +this is why — not a bug in the screen. + +## Response shapes + +Top-level wrapper is always `{"meals": ...}` — except `categories.php`, which uses `{"categories": [...]}`. + +**`filter.php` returns summaries, everything else returns full records.** Modelled as two DTOs in +`network/.../dto/RecipeDetailsDto.kt`: + +- `RecipeBasicDto` — `idMeal`, `strMeal`, `strMealThumb` only. +- `RecipeFullDto` — adds category, area, instructions, youtube, source, tags, and the ingredients. + +**Never present a filter result as a complete recipe.** Filter to get candidates, then `lookup.php?i=` +each `idMeal` for the real ingredients and instructions. This is TheMealDB's own documented workflow, +and it is also just true of the data — a summary has no instructions to show. + +## DTO field traps + +- **Ingredients are flat, not a list.** `strIngredient1..20` paired with `strMeasure1..20` by index. + Pair by `N`, keep order, trim, skip null/empty ingredient slots. **A measure can be blank while its + ingredient exists** — don't drop the ingredient. Already handled in `RecipeFullDto`. +- **`{"meals": null}` means not found**, not an error. Expect it from any search or filter. +- **`meals` is not always an array.** The v2 schema allows a string or object in that slot for + access-level messages. `Json { ignoreUnknownKeys = true }` does **not** save you here — it tolerates + unknown *keys*, not a changed *type*, so such a response throws. +- **`idMeal` is typed `integer | string`** in the spec. It arrives as a string in practice; the repo + wraps it as `RecipeId`. +- **`strThumb` on ingredients is a lie.** `IngredientDto` declares it, but `list.php?i=list` does not + return it. Build ingredient images from the name instead (see `/themealdb-api` → Images). +- **`strCountry` on the area list is nullable** and absent on some rows, so `AreaDto.country` is + `String?`. A non-null declaration fails the decode of the *entire* list on one bad row. +- Nullable in practice: `strArea`, `strCategory`, `strTags`, `strYoutube`, `strSource`, + `strImageSource`, `strCreativeCommonsConfirmed`, `dateModified`. diff --git a/.claude/rules/storage.md b/.claude/rules/storage.md new file mode 100644 index 0000000..9985cb0 --- /dev/null +++ b/.claude/rules/storage.md @@ -0,0 +1,19 @@ +--- +paths: + - "storage/**" +--- + +# Local storage (Room 3 + DataStore) + +**Room has no destructive-migration fallback, deliberately.** Changing an entity without bumping +`AppDatabase.version` throws "Room cannot verify the data integrity" on first db access; bumping +the version without writing a `Migration` throws "A migration from N to M was required but not +found." Both are intentional — the crash is the reminder to write the migration. Don't "fix" one +by adding `fallbackToDestructiveMigration`; note that it would not even cover the first case, +since `checkIdentity` runs in `onOpen`, before any migration path. + +The db is `recipe.db` on every platform (one `DATABASE_NAME` constant in `StorageFactory.kt`). +Per-platform delete commands: `README.md` → *Resetting local data*. + +Serialize stored JSON with the `@StorageJson` qualifier, not `@NetworkJson` +(`core/.../serialization/JsonQualifiers.kt`). diff --git a/.claude/skills/circuit-screen/SKILL.md b/.claude/skills/circuit-screen/SKILL.md index 1a68b9f..d7bc39d 100644 --- a/.claude/skills/circuit-screen/SKILL.md +++ b/.claude/skills/circuit-screen/SKILL.md @@ -43,25 +43,10 @@ sealed interface XxxEvent { @CircuitSerializable(AppScope::class) data object XxxScreen : Screen ``` -Rules that are easy to get wrong: - -- **`@Redacted` on every `eventSink`.** Comes from `dev.zacsweers.redacted.annotations.Redacted`; - keeps lambdas out of `toString`. -- **Events are nested per state**, not one flat sealed interface. A `Success` state's sink only - accepts `XxxEvent.Success`. -- **`retain { }`**, from `androidx.compose.runtime.retain`, for state that must survive - config change — never `rememberSaveable`. -- **`@CircuitSerializable(AppScope::class)` on the `Screen`**, from - `com.slack.circuit.serialization`. It is `@MetaSerializable`, so it already implies - `@Serializable` — don't add that too. Screens stopped being `Parcelable` in Circuit 0.38; - `@Parcelize` now belongs only to the `:model` id value classes. -- A retry is conventionally an `Int` counter (`retryTrigger`) passed into the producer as a key, - incremented by `XxxEvent.Error.RetryClicked`. -- Screens with parameters are a sealed interface whose **concrete cases** carry the annotation — - see `RecipesScreen.ByCategory` / `.ByArea` / `.BySearch` / `.Favorites`. The sealed parent is - never annotated. -- Every parameter of a screen must be serializable. `RecipeId`, `CategoryId` and `IngredientId` - already are; a new parameter type in `:model` needs `@Serializable` added there. +The rules this shape must obey — `@Redacted` sinks, per-state events, `retain { }`, +`@CircuitSerializable` and what it implies, parameterised screens, serializable parameters — live +in `.claude/rules/circuit.md`, which loads whenever you touch `:domain` or `:ui`. Read it before +reviewing a new presenter. ### Turning repository data into screen state @@ -81,26 +66,14 @@ return state.foldToState( `Success(isRefreshing = true)` rather than dropping back to `Loading`. Do not add a `retain`ed var beside it to do that job — that pattern predates `ContentState` and is gone. -Inside `foldToState`, **`hasAnswer` is the discriminator, not `data.isEmpty()`**: an empty list is a -real answer, and reading it as "nothing yet" hangs a spinner over an empty screen. The one empty list -that isn't an answer is an empty read from cache while its request is still in flight. The -repository holds it back (`asOutcomes(fetching = refresh) { … }`) and `hasAnswer` refuses it. See the -Architecture section of `CLAUDE.md`. +Inside `foldToState`, **`hasAnswer` is the discriminator, not `data.isEmpty()`** — see +`.claude/rules/data-flow.md`. ### Screen persistence -The back stack is saved with kotlinx-serialization. Circuit codegen turns each -`@CircuitSerializable` screen into a `CircuitSerializerRegistration` multibinding, and -`domain/.../circuit/CircuitProviders.kt` folds the whole set into a `SerializableCircuitSaver` on -`Circuit.Builder`. Nothing to register by hand — but the failure modes are runtime, not compile -time: - -- **Saving** an unregistered or unserializable screen throws. That is the loud, useful case. -- **Restoring** one can only return null, and the nav stack drops that record; `CircuitProviders` - passes `onRestoreError` to the logger so it isn't silent. - -`SubScreen`s from `circuitx-subcircuit` never enter the back stack and get **no** annotation — they -may hold unserializable state such as a `TextFieldState`. +Nothing to register by hand: codegen turns each `@CircuitSerializable` screen into a saver +registration. The runtime failure modes, and why `SubScreen`s get no annotation, are in +`.claude/rules/circuit.md` → *Navigation persistence*. ## 2. Producer — `domain/.../producer/XxxProducer.kt` diff --git a/.claude/skills/themealdb-api/SKILL.md b/.claude/skills/themealdb-api/SKILL.md index 8384f17..b6a6fb5 100644 --- a/.claude/skills/themealdb-api/SKILL.md +++ b/.claude/skills/themealdb-api/SKILL.md @@ -1,29 +1,15 @@ --- name: themealdb-api -description: TheMealDB API reference for this repo — v2 base URL and key handling, every endpoint and which @Resource maps to it, free-vs-premium tier limits, response shapes and DTO field traps, image URL construction, and attribution rules. Use when adding or changing a network call, debugging an empty or truncated API response, mapping a DTO, or building an image URL. +description: TheMealDB API reference for this repo — every endpoint and which @Resource maps to it, free-vs-premium tier limits, image URL construction, and attribution rules. Use when adding a network call, choosing an endpoint, debugging an empty or truncated API response, building an image URL, or writing user-facing copy about recipe data. DTO field traps and key handling are in .claude/rules/network.md. --- # TheMealDB API -This app uses **v2 exclusively**. The v1 fallback was removed — see [Why v2 only](#why-v2-only). +On-demand reference for the endpoint map, tier limits, images, and attribution. The invariants — +base URL and key handling, why v2 only, response shapes, and DTO field traps — are in +`.claude/rules/network.md`, which loads whenever you touch `:network` or `:repository`. -## Base URL and key - -``` -https://www.themealdb.com/api/json/v2/{MEALDB_API_KEY}/ -``` - -Built in exactly one place: `network/.../NetworkProviders.kt` (`MEALDB_BASE_URL` + `DefaultRequest`). -Every `@Resource` path is relative to that trailing slash, so no resource declares a version. - -`MEALDB_API_KEY` is **required**. It flows Gradle property or env var → -`build-logic/.../ProjectExt.kt` → BuildKonfig `SharedBuildConfig.MEALDB_API_KEY` (`shared/build.gradle.kts`) -→ each platform's `RuntimeConfigImpl` → `RuntimeConfig.mealDbApiKey: String` (non-null). A missing or -blank key fails the Gradle build with a message pointing at the signup page. - -**The key is in the URL path, not a header.** No `LogLevel` short of `NONE` omits the request URL, so -`NetworkProviders.redacting()` wraps the Ktor logger and rewrites the key to `***`. Preserve that -wrapper if you touch the logging block, and never add a second client that logs API URLs unredacted. +Base URL: `https://www.themealdb.com/api/json/v2/{MEALDB_API_KEY}/` (v2 only, key in the path). ## Endpoints @@ -51,12 +37,7 @@ repository. Adding one is a normal change — mirror an existing sibling in `res Spaces in `c` / `a` / `i` values may be written as underscores (`chicken_breast`). URL-encode anything user-supplied. `filter.php?i=` accepts **up to four** comma-separated ingredients on v2. -**`list.php?a=list` and `filter.php?a=` do not agree on what an area is.** The list returns ~195 -rows with a real key (see the tier table below), but `filter.php` matches the exact `strArea` -string meals actually carry — 14 distinct values on v1. Most rows in the area list therefore have -no recipes behind them. `RecipeApiImpl.getByArea` asks for both spellings the list gives -(`Italian` and `Italy`) and merges the results, which recovers some but not all of the gap. If -the Areas tab looks full of dead ends, this is why — not a bug in the screen. +`list.php?a=list` and `filter.php?a=` disagree on what an area is — see `.claude/rules/network.md`. Full spec: `reference/openapi-v2.yaml` (vendored 2026-08-26). @@ -75,46 +56,7 @@ Measured against the live API, dev key `1` vs a premium key. This is the non-obv | `popular.php` | 404 | 20 | 20 | | `latest.php` / `randomselection.php` | Patreon message | 1 | 10 | -### Why v2 only - -With a real key, v2 returns everything v1 does plus more results and four extra endpoints — v1 has -no advantage. But **v2 with the free dev key silently caps `filter.php` at one result**, which would -reduce every browse-by-category screen to a single recipe and look like an app bug rather than a -tier limit. Hence: one base URL, and a build that fails without a key rather than shipping that. - -If a filter suddenly returns exactly one meal, suspect the key before suspecting the code. - -## Response shapes - -Top-level wrapper is always `{"meals": ...}` — except `categories.php`, which uses `{"categories": [...]}`. - -**`filter.php` returns summaries, everything else returns full records.** Modelled as two DTOs in -`network/.../dto/RecipeDetailsDto.kt`: - -- `RecipeBasicDto` — `idMeal`, `strMeal`, `strMealThumb` only. -- `RecipeFullDto` — adds category, area, instructions, youtube, source, tags, and the ingredients. - -**Never present a filter result as a complete recipe.** Filter to get candidates, then `lookup.php?i=` -each `idMeal` for the real ingredients and instructions. This is TheMealDB's own documented workflow, -and it is also just true of the data — a summary has no instructions to show. - -### Field traps - -- **Ingredients are flat, not a list.** `strIngredient1..20` paired with `strMeasure1..20` by index. - Pair by `N`, keep order, trim, skip null/empty ingredient slots. **A measure can be blank while its - ingredient exists** — don't drop the ingredient. Already handled in `RecipeFullDto`. -- **`{"meals": null}` means not found**, not an error. Expect it from any search or filter. -- **`meals` is not always an array.** The v2 schema allows a string or object in that slot for - access-level messages. `Json { ignoreUnknownKeys = true }` does **not** save you here — it tolerates - unknown *keys*, not a changed *type*, so such a response throws. -- **`idMeal` is typed `integer | string`** in the spec. It arrives as a string in practice; the repo - wraps it as `RecipeId`. -- **`strThumb` on ingredients is a lie.** `IngredientDto` declares it, but `list.php?i=list` does not - return it. Build ingredient images from the name instead (below). -- **`strCountry` on the area list is nullable** and absent on some rows, so `AreaDto.country` is - `String?`. A non-null declaration fails the decode of the *entire* list on one bad row. -- Nullable in practice: `strArea`, `strCategory`, `strTags`, `strYoutube`, `strSource`, - `strImageSource`, `strCreativeCommonsConfirmed`, `dateModified`. +Why that forces v2 with a real key: `.claude/rules/network.md` → *Why v2 only*. ## Images diff --git a/.claude/skills/verify/SKILL.md b/.claude/skills/verify/SKILL.md index 6ff8825..e21d733 100644 --- a/.claude/skills/verify/SKILL.md +++ b/.claude/skills/verify/SKILL.md @@ -64,7 +64,7 @@ reports and fails on a class inferred `unstable` or a `restartable` composable t Run it as its own invocation, never folded into `./gradlew build ...`: every target of a module writes over the same reports directory, and this task's single designated compile task is what makes the result deterministic. The failure names each offender and the allowlist line to add. -Prefer fixing the type over allowlisting — CLAUDE.md, *Compose stability*, says which annotation +Prefer fixing the type over allowlisting — `.claude/rules/compose-stability.md` says which annotation belongs where. ### 5. Build each touched module diff --git a/CLAUDE.md b/CLAUDE.md index d862d4f..c8036b2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,29 +4,20 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Kotlin Multiplatform recipe app targeting Android, iOS, Desktop (JVM), JS, and WasmJS. -## Build & verify +Area-specific guidance lives in `.claude/rules/` and loads automatically when you touch matching +files: `gradle-build`, `hot-reload`, `data-flow`, `compose-stability`, `circuit`, `network`, +`storage`. Read the relevant one directly if you need it before opening a file in that area. -Gradle 9.7.1. Everything runs on **JDK 25** and everything compiles to **JVM 17** — those are two -different settings and the split is deliberate. The daemon uses Amazon Corretto 25, auto-provisioned -via foojay from `gradle/gradle-daemon-jvm.properties`; `jvm-toolchain = "25"` in the catalog is the -JDK that compiles the code and runs it (`:desktopApp:run`, tests, the jpackage runtime image), while -`jvmSourceCompatibility`/`jvmTargetCompatibility = "17"` is the bytecode and API level. -Don't override the JDK. +## Build & verify -A toolchain silently sets `jvmTarget` to its own version unless a target pins one, so **every JVM -compilation pins it explicitly** — `jvm {}` and `android {}` in `kmp.library`, plus `:app` and -`:desktopApp` — each with `jvmTarget` *and* `-Xjdk-release=17`, which additionally hides post-17 JDK -APIs so nothing that would crash on Android can compile. Drop either and you silently get bytecode -25. `-Xjdk-release` is JVM-only: never add it to the project-wide `compilerOptions`, which also -feeds Native, JS, and WasmJS. `kotlin.jvm.target.validation.mode` is unset (default `error`), so a -Kotlin/Java target mismatch fails the build rather than shipping — that check is a feature, not an -obstacle. +Gradle 9.7.1. Everything runs on **JDK 25** and compiles to **JVM 17** — the split is deliberate; +don't override the JDK. Why, and how every JVM compilation pins it: `.claude/rules/gradle-build.md`. - **Compile check after edits:** `./gradlew ::build`. Builds every target for that module, which catches `expect`/`actual` mismatches and web/native breakage that a single-target compile misses. - **Tests:** `./gradlew :domain:jvmTest` — `:domain` is the only module with tests today. Aggregate: `./gradlew allTests`. - **Run:** `:desktopApp:run` · `:webApp:wasmJsBrowserDevelopmentRun` · `:webApp:jsBrowserDevelopmentRun` · `:app:installDebug`. - For desktop UI work prefer `:desktopApp:hotRun` — see *Compose Hot Reload* below. + For desktop UI work prefer `:desktopApp:hotRun` — see `.claude/rules/hot-reload.md`. CI (`.github/workflows/build.yml`, ubuntu) runs `ktfmtCheck checkSortDependencies`, `detektAll` and `build` on every PR and push to `main`; it needs the `MEALDB_API_KEY` repository secret. It only @@ -39,65 +30,6 @@ reports, and Linux skips linking the iOS frameworks, so still run this loop befo ./gradlew ::build ``` -- `detektAll` is a custom aggregate task from `build-logic/src/main/kotlin/detekt.gradle.kts`. - Plain `detekt` does not cover all source sets. -- `buildHealth` (dependency-analysis) is noisy about Compose artifacts. Most modules already carry - `dependencyAnalysis { issues { onUnusedDependencies { exclude(...) } } }` — add to those rather - than deleting a dependency it flags. - -## Compose Hot Reload - -Swaps changed classes into the running desktop app instead of restarting it. **Nothing in the build -declares it** — the Compose Multiplatform Gradle plugin (1.12.0) auto-applies -`org.jetbrains.compose.hot-reload` (1.2.0) to every project with the Kotlin JVM or Multiplatform -plugin, so there is no alias in `libs.versions.toml` to bump and nothing to look for in -`desktopApp/build.gradle.kts`. `-Porg.jetbrains.compose.hot.reload.disable=true` turns it off. -The `exclude("org.jetbrains.compose.hot-reload:hot-reload-runtime-api")` lines in `:desktopApp`, -`:ui`, and `:domain` are dependency-analysis reacting to the artifact the plugin injects. - -`gradle.properties` sets `compose.reload.jbr.autoProvisioningEnabled=true`: hot reload needs a -JetBrains Runtime for enhanced class redefinition, and no JBR is installed system-wide, so Gradle -fetches one through the foojay resolver on first use (~200MB, once). It follows the project -toolchain, so it now provisions **JBR 25** (`jbrsdk_jcef-25.0.4.1`) and the daemon, the compile -toolchain, and the hot-reload JVM are all 25. An older `jbrsdk_jcef-21` may still be sitting in -`~/.gradle/jdks` from before the toolchain moved; it is unused. - -``` -./gradlew :desktopApp:hotRun --auto # continuous build; reloads on save -./gradlew :desktopApp:hotRun # explicit mode; reload with ./gradlew :desktopApp:reload -``` - -`:desktopApp` is a plain `kotlin("jvm")` module, so its tasks have **no target suffix** — -`hotRun`, `hotRunAsync`, `hotMcpServer`, `reload`. The KMP modules that apply Compose (`:ui`, -`:domain`, `:shared`) get their own `hotRunJvm`/`hotMcpServerJvm`; you never want those, and it is -why `.mcp.json` names `:desktopApp:hotMcpServer` in full rather than the bare `hotMcpServer` the -upstream README suggests — the short form matches every one of those projects at once. - -Editing a composable in `:ui` reloads into the `:desktopApp` window; cross-module reload works -because both are in the same build. - -**Reload output does not appear in the `hotRun` terminal.** `--auto` forks a second Gradle daemon for -the continuous build, and its recompile/reload log goes to `desktopApp/build/run/main/main.chr.log` — -read that to see "Change detected", the build result, and which `@Composable` scopes were invalidated. -The staged classes land in `desktopApp/build/run/main/classpath/hot/`. The `hotRun` terminal only -carries the app's own stdout. If a reload seems not to land, check that log before assuming the app -is wedged, and check for a stale app process from an earlier session — each `hotRun` starts its own. - -**MCP server.** `.mcp.json` registers `compose-hot-reload`, which exposes `status`, `reload`, -`await_reload`, `take_screenshot`, `get_semantic_tree`, `get_ui_error`, `get_logs`, `list_windows`, -`click`, `type_text`, `scroll`, `scroll_to_index`, `resize_window`, `restart`, and `reset_ui`. -The agent starts the server itself; it waits for the app and connects when it appears, so launch -`hotRun` separately. Check `status.buildContinuous` to pick between `reload` (explicit mode) and -`await_reload` (started with `--auto`). - -Two limits worth knowing here: - -- **The Metro graph is not reloaded.** `createGraph()` runs outside the composition - in `main.kt`, so DI and provider changes need `restart`, not `reload`. -- **Retained state survives a reload.** `retain { }` and presenter state persist across a swap — - usually what you want, but a state-shape change can leave stale values behind. `reset_ui` discards - the composition; `restart` restarts the process. - ## Delegation Project agents in `.claude/agents/` run on cheaper models. Prefer them over doing the work inline @@ -120,24 +52,12 @@ main session; delegate the execution and the summarizing. subprojects by the `formatting` convention plugin. Never hand-format — run `ktfmtFormat`. - Detekt config lives at `config/detekt/detekt.yml` (built on defaults) plus `io.nlopez.compose.rules`. The IDE plugin treats findings as errors. -- `-Xexpect-actual-classes` is set project-wide by `kmp.library`. Don't re-declare it per file. - The `-opt-in=kotlin.time.ExperimentalTime` that used to sit beside it is gone: `kotlin.time.Clock` - and `Instant` went Stable in Kotlin 2.3.0, so neither the flag nor a per-file `@OptIn` is needed. -## Module & build conventions +## Modules and source sets -- New library modules apply the **`kmp.library`** convention plugin - (`build-logic/src/main/kotlin/kmp.library.gradle.kts`), not raw KMP plugins. It sets the targets, - toolchain, Android namespace (`com.scottolcott.recipe.`), ktfmt, detekt, and sort-dependencies. - Targets: `android`, `jvm`, `iosArm64`, `iosSimulatorArm64`, `js(browser, ESM)`, `wasmJs(browser, ESM)`. - **No `iosX64`** — Intel Macs can't run the iOS simulator build. - Custom source-set groups available: `commonJvm` (jvm+android), `web` (js+wasmJs), `nonWeb`, `nonAndroid`. - Typesafe project accessors are enabled: write `projects.domain`, not `project(":domain")`. -- `@Parcelize` comes from `io.github.solcott.kmp.parcelize` (a multiplatform plugin), **not** - kotlin-parcelize. It applies only to the `:model` id value classes (`RecipeId`, `CategoryId`, - `IngredientId`) — **`Screen`s are not Parcelable**, see *Navigation persistence* below. -- Any module declaring `Screen`s needs the kotlinx-serialization Gradle plugin - (`libs.plugins.kotlinx.serialization`); `:domain` already applies it. ## Architecture @@ -148,195 +68,25 @@ Layering is strict. Respect it: | Presentation logic | `:domain` | Circuit `Presenter`, `CircuitUiState`, events, `Screen`, producers | | Composables | `:ui` | `@Composable` screen UI only | | Data | `:repository` | Store 5 repositories returning `Flow>` | -| Remote | `:network` | Ktor 3 with typed `ktor-client-resources`, DTOs — see `/themealdb-api` for endpoints, tiers, and DTO traps | +| Remote | `:network` | Ktor 3 with typed `ktor-client-resources`, DTOs — see `/themealdb-api` for endpoints and tiers | | Local | `:storage` | Room 3 (`androidx.room3`) + DataStore | A presenter and its UI live in **different modules**, joined only by `@CircuitInject(XxxScreen::class, AppScope::class)` on each side. See `/circuit-screen` for the full pattern when adding a screen. -### Reading data: `Outcome` and `ContentState` - -Repositories return `Flow>`, not Store's own `StoreReadResponse`. **Store 5 is an -implementation detail of `:repository`** — it is an `implementation` dependency there, and nothing -above that module compiles against it. `io.github.solcott:dataresult-store5` does the translation -in `asOutcomes(fetching = refresh) { … }`, at the end of each repository's chain. It sits *inside* -the `flatMapLatest { refresh -> … }` so it can see whether a fetch is coming. - -The types come from [kmp-dataresult](https://github.com/solcott/kmp-dataresult), shared with the -`Countries` project. Resolving them needs `gpr.user`/`gpr.key` in `~/.gradle/gradle.properties` (a -classic PAT with `read:packages`) — GitHub Packages authenticates even public reads. - -The path from a repository to a screen state is three steps: - -1. `Flow>` — `Loading`, `Data(value, origin)` or `Error(cause, origin)`. Store's - `Initial` and `Loading` both become `Outcome.Loading`; `NoNewData` is dropped. An empty first - read from cache while a fetch is pending is held back until the fetch answers. Room returns `[]` - for a key it has never fetched, and that's a cache miss, not an empty result. - `getFavoritesAsFlow` passes no `fetching`, because nothing is coming to answer instead. -2. `produceRetainedContentState(initial, keys) { … }` from `libs.uistateCircuit` - (`io.github.solcott.uistate.circuit`) folds those emissions into a retained `ContentState`. - **`ContentState.data` is what holds the last loaded value** — the separate retained - `lastItems`/`lastRecipes` caches that used to sit beside the response are gone, and so is - `ListUi`. Shared with `Countries`; the local copy in `:domain`'s `producer` package is gone. -3. `ContentState.foldToState(onLoading, onError, onContent)` in `presenter/ListContent.kt` maps it - onto a screen's own three-case `CircuitUiState`. - -A screen fed by several sources at once uses the group versions of the first two steps. The -repository returns `combineOutcomes(a, b, c)`, a `Flow>` with each source seeded -with `Loading` so the fastest one shows at once. The presenter folds it with -`produceRetainedContentStates(contentStatesOf(…))` into one `ContentState` per source. Destructure -the group for names; `isAnyLoading`/`errorOrNull` on it answer for all of them. `SearchPresenter` is -the example. - -Two things are easy to get wrong: - -- **Check `hasAnswer`, never `data.isEmpty()`**, to tell "nothing has loaded yet" from "loaded and - empty". `hasLoaded` reads `origin`, which stays null until the first value arrives. `hasAnswer` - also refuses an empty read *from cache* while its request is in flight or has failed. That's a key - the database has never seen, not an empty result. The repositories already hold that read back - with `asOutcomes(fetching = refresh) { … }`, and `hasAnswer` is the backstop. Testing the data for - emptiness leaves a spinner over a legitimately empty tab forever. `hasAnswer` can't, because it is - only false while a request is outstanding or has failed. -- **Having data beats being in flight.** `foldToState` checks `hasAnswer` first, which is what - renders a background refresh as `isRefreshing` over the existing grid instead of dropping back to - a full-screen spinner. `domain`'s `refreshKeepsPreviousAreas` test pins this — don't reorder those - branches. - -`DataError` is structured; screen states carry `String`. `DataError.toMessage()` in -`ListContent.kt` bridges the two, and belongs in `:ui` with the rest of the user-facing copy once -these get localized. - -### Compose stability - -Everything a presenter hands to a composable has to be *stable*, or Compose re-runs the UI on every -recomposition. Two rules keep it that way: - -- **Collections headed for the UI are `ImmutableList`**, from the repository's public signature - onward — `SourceOfTruth.reader`, `toModel()`, the `Flow>` return type, the producer's - `ContentState>`, and the screen state's field. Entities and DTOs stay `List`: - Store's local type is `List` and nothing below `toModel()` ever reaches composition, so - converting there is a copy that buys nothing. `:model` carries - `api(libs.kotlinx.collections.immutable)` for this. -- **`@Immutable` when it is true, `@Stable` when it is not.** `@Immutable` promises no public - property ever changes: right for the `:model` classes and for a screen state whose fields are all - values (`AreasState`, `RecipesState`, `RecipeDetailsState`, `SearchTabState`, `RecipesScreen`). - A state that carries an observable holder — `SearchBarState` and `TextFieldState` in `SearchState`, - `NavStack`/`Navigator` in `RecipeScaffoldState` and `HomeState`, the Metro graph in - `DesktopAppGraph` — gets `@Stable`. Both make the type skippable; only one of them is honest, and - a class marked `@Stable` must back its mutable properties with snapshot state (this is why - `BackShortcutHost.onBack` is a `MutableState`). - -Annotate the *declared parameter type*: `@Stable` on a supertype does not propagate, so -`DesktopAppGraph` carries its own annotation rather than inheriting one from `AppGraph`. - -A sealed `CircuitUiState` interface is unstable until annotated — the compiler cannot see the -implementations — so every one of them carries a marker. - -**None of the above is taken on trust — `./gradlew composeStabilityCheck` enforces it.** The -`compose.stability` convention plugin turns on the Compose compiler's own metrics and reports for -the six modules that apply the compiler plugin (`:ui`, `:domain`, `:shared`, `:app`, `:webApp`, -`:desktopApp`) and registers a per-project check that reads them. It fails on a class the compiler -inferred as `unstable`, and on a `restartable` composable that is not `skippable`. A `runtime` -class passes: its stability depends on a generic argument and is settled at run time. There is no -aggregate task — Gradle's cross-project name matching fans `composeStabilityCheck` out to all six. - -Exceptions live in `config/compose/unstable-classes.txt` and -`config/compose/unskippable-composables.txt`, one `:` per line, each -with a comment saying why. Today only the Metro graph impls and the two Android entry points are -listed, none of which is ever a composable parameter. - -Reports land in `/build/reports/compose/reports/` (`-classes.txt`, `-composables.txt`) with -`-module.json` counts under `.../metrics//main/`. Read them for *why* something failed — -the entry lists each property or parameter and its verdict. The compiler names them after the -Kotlin module, which is the Gradle project path — so they carry a colon, `Recipes:domain-classes.txt`, -and CI stages a renamed copy before uploading them because `upload-artifact` rejects one. - -Two traps in those reports: - -- **Only the metrics path is per-target; the reports path is not.** The Compose plugin appends - `/` to `metricsDestination` but hands `reportsDestination` to every - compilation verbatim, and a KMP module's targets share a Kotlin module name. So all six targets - write the same `Recipes_ui-classes.txt` and the last to run wins. After a full `./gradlew build` - you are reading whichever target finished last. `composeStabilityCheck` is the invocation to - trust: it depends on exactly one compile task (JVM where there is one, `compileDebugKotlin` for - `:app`, `compileKotlinJs` for `:webApp`), so it has a single writer. That is also why it is not - wired into `check`, and why CI runs it as its own step *before* `build`. -- **`reportsDestination` is a compiler *input*, not an output.** A compile task whose reports have - been deleted still counts as up to date and quietly writes nothing. `compose.stability` declares - the directory as an output of that one designated compile task to fix it, which also means the - task re-runs after a full `build` and stays the last writer. - -Detekt's `UnstableCollections` covers the same ground at the source level and is on. It is right -about composables that emit UI and wrong about ones that return a value — those are neither -restartable nor skippable, so a parameter's stability is inert. `RecipesProducer.produceByIngredients` -is the single `@Suppress` for that reason. - -**A test fake for an in-flight request must not complete.** `produceRetainedContentState` settles a still -loading status when its source completes, so a spinner can never hang — which means -`flowOf(Outcome.Loading)` is a *settled empty result*, not a pending one. Store streams never -complete; use the `inFlight()` helper in the presenter tests, which emits `Loading` and then awaits -cancellation. - -**Circuit screen file layout** (`domain/.../presenter/XxxPresenter.kt`), in this order: -presenter class → sealed `XxxState : CircuitUiState` (Loading/Error/Success) → sealed `XxxEvent` -with nested per-state sub-interfaces → the `Screen` last, as -`@CircuitSerializable(AppScope::class) data object XxxScreen : Screen`. -`eventSink` properties are annotated `@Redacted` so they stay out of `toString`. - **DI is Metro** (`dev.zacsweers.metro`), compile-time. Each layer exposes an `XxxProviders` interface annotated `@ContributesTo(AppScope::class)`; implementations use `@ContributesBinding(AppScope::class)` + `@SingleIn(AppScope::class)` + `@Inject`. Per-platform graphs (`AndroidAppGraph`, `DesktopAppGraph`, `IOSAppGraph`, `WebAppGraph`) all implement `shared/.../AppGraph.kt`. **There is no runtime container to register into — wire via annotations.** -- State survives recomposition/config change via `androidx.compose.runtime.retain.retain { }`, - **not** `rememberSaveable`. One sanctioned exception: `HomePresenter` persists the selected tab - with `rememberSerializable`, because that selection is cheap to store and worth surviving process - death so the app reopens on the tab the user left. Don't "fix" it to `retain { }`. -- A new screen must also be added to `domain/.../navigation/ScreenUrlMapper.kt` — both the - `Screen.toUrlPath()` and `urlPathToScreen()` directions. It backs `recipes://app/...` deep links - and browser history on web. Home tabs are the exception to writing anything there by hand: each - `HomeTabScreen` carries its own `urlSegment` and the mapper reads `HOME_TABS`, so adding a tab to - that one list makes it addressable in both directions. -- **`:domain` depends on Material3 (`api`), deliberately.** `SearchState` holds a `SearchBarState` - and `RecipeScaffoldState`/`SearchScreen` carry a `SearchBarValue`, so that the presenter — not the - composable — owns whether the search bar is expanded. This is the *only* sanctioned place for - Material3 widget state in `:domain`; other widget state belongs in `:ui`. -- Two `Json` qualifiers exist, `@NetworkJson` and `@StorageJson` - (`core/.../serialization/JsonQualifiers.kt`). Pick deliberately. - -**Navigation persistence** (Circuit 0.38+): `Screen` and `PopResult` are no longer `Parcelable`, so -the back stack is persisted with kotlinx-serialization. See `/circuit-screen` for the -`@CircuitSerializable` rules and the saver wiring. - ## Gotchas - `MEALDB_API_KEY` (Gradle property or env var) is **required** — the build fails without it. - It's read at build time by `build-logic/.../ProjectExt.kt` and baked into `SharedBuildConfig` via - BuildKonfig. The app talks to TheMealDB **v2 only**; there is no v1 fallback, because v2 on the - free dev key silently caps `filter.php` at one result. The key is interpolated into the URL - *path*, so `NetworkProviders.redacting()` keeps it out of Ktor's logs — don't remove that wrapper. - Details and the full endpoint reference: `/themealdb-api`. -- **Room has no destructive-migration fallback, deliberately.** Changing an entity without bumping - `AppDatabase.version` throws "Room cannot verify the data integrity" on first db access; bumping - the version without writing a `Migration` throws "A migration from N to M was required but not - found." Both are intentional — the crash is the reminder to write the migration. Don't "fix" one - by adding `fallbackToDestructiveMigration`; note that it would not even cover the first case, - since `checkIdentity` runs in `onOpen`, before any migration path. - The db is `recipe.db` on every platform (one `DATABASE_NAME` constant in `StorageFactory.kt`). - Per-platform delete commands: `README.md` → *Resetting local data*. -- **The nav stack is `rememberSaveableNavStack`, not `rememberSaveableBackStack`.** `BackStack` - stubs `forward()` and `backward()` to `false`, and `NavigatorImpl` delegates straight to them, so - switching to it silently turns the browser back/forward buttons on web into no-ops — - `ui/src/webMain/.../BrowserHistoryEffect.web.kt` drives navigation through - `Navigator.backward()`/`forward()`. Both `RecipeApp.kt` and `RecipeScaffoldPresenter.kt` must stay - on `rememberSaveableNavStack`. -- The circuit saver is installed with the static `setCircuitSaver(saver)` overload rather than the - `setCircuitSaver { fallback -> ... }` transform, so an unregistered screen **fails loudly at save - time** instead of quietly falling through to the registry-backed saver. That is deliberate. - Restoring an unregistered screen can only return null and drop the record, so `CircuitProviders` - logs it through `onRestoreError`. + How it flows and why it is redacted from logs: `.claude/rules/network.md`. +- Recipe data and images are TheMealDB's: credit it in anything user-facing, and never infer that + a recipe is allergen-free, vegetarian, halal, etc. — `/themealdb-api` → *Attribution*. - `mavenLocal()` is in the repository list because `io.github.solcott:kmp-parcelize` is sometimes published locally. If it fails to resolve, that's why. - `README.md`'s iOS instructions are stale: the file on disk is `iosApp/iosApp.xcodeproj`, not `.xcworkspace`.