feat: module boundary guard, and two API-level bugs it uncovered
checkModuleBoundaries holds the dependency tables in docs/architecture/README.md as a check: every module's permitted project dependencies, plus the rule that domain:cycle and domain:prediction must never apply an Android plugin. core:ads is already in the map with an empty permitted set, before the module exists — PRODUCT_PLAN.md §34 is non-negotiable, and a guard written alongside the code it constrains is one shaped around whatever exception somebody wanted at the time. It lists every violation rather than the first, and refuses to report a pass when it examined no modules at all. THE GUARD FAILED ITS OWN FIRST PROOF prove-guard.sh injected a forbidden dependency into :domain:prediction and the guard reported "7 modules checked, no violations". The root project is configured before its subprojects, so reading subprojects.configurations from the root script saw every configuration empty — it had been green over an empty map since the moment it was written, and would have been trusted for months. Collection moved into afterEvaluate, and the task now throws rather than passing if it ends up with no modules. Three proofs recorded in the architecture doc, all re-run and all red: a domain module reaching upward, :app reaching past the repository straight to Room, and a module with no rule being reported as unmeasured rather than assumed fine. TWO REAL BUGS FROM WIRING IT INTO `check` Running the whole check for the first time turned up Android lint errors that would have shipped: NewApi: java.time.LocalDate#ofInstant requires API 34 (minSdk is 26) NewApi: java.time.LocalDate#EPOCH requires API 34 (minSdk is 26) Both are on the recalculation path. On any device below Android 14 — most of the install base this app targets — that is a crash. Neither the unit tests nor the API 36 emulator could see it; lint is the only thing that could. Replaced with atZone().toLocalDate() and ofEpochDay(0), which are API 26. Also cleared the lint warnings that were real: a redundant activity label, and a round launcher icon declared but never referenced. The two that remain are deliberate and now say so where the warning is read — targetSdk 36 is Play's floor and raising it opts into untested runtime behaviour, and the -v26 mipmap qualifier stays because removing it makes AAPT fail to resolve the icon at all. ./gradlew check now passes with 0 lint errors across all seven modules. 70 unit tests, all passing. closes #7
This commit is contained in:
null
parent
adc50751d8
commit
f5e9fbe53c
|
|
@ -19,6 +19,10 @@ android {
|
|||
minSdk = 26
|
||||
// Google Play requires API 36 for new apps and updates from 2026-08-31.
|
||||
// Confirm the current requirement at submission time rather than here.
|
||||
// Lint warns (OldTargetApi) that 37 exists. Staying at 36 is the
|
||||
// decision: it is Play's floor, and raising targetSdk opts the app into
|
||||
// runtime behaviour changes that nothing here has been tested against.
|
||||
// Revisit with a QA round, not with a lint fix.
|
||||
targetSdk = 36
|
||||
versionCode = 1
|
||||
versionName = "0.1.0"
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@
|
|||
android:dataExtractionRules="@xml/data_extraction_rules"
|
||||
android:fullBackupContent="false"
|
||||
android:icon="@mipmap/ic_launcher"
|
||||
android:roundIcon="@mipmap/ic_launcher_round"
|
||||
android:label="@string/app_name"
|
||||
android:supportsRtl="true"
|
||||
android:theme="@style/Theme.Period">
|
||||
|
|
@ -24,7 +25,6 @@
|
|||
<activity
|
||||
android:name=".MainActivity"
|
||||
android:exported="true"
|
||||
android:label="@string/app_name"
|
||||
android:theme="@style/Theme.Period">
|
||||
<intent-filter>
|
||||
<action android:name="android.intent.action.MAIN" />
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ data class TodayUiState(
|
|||
val cycles: List<CycleRecord> = emptyList(),
|
||||
val forecast: Prediction? = null,
|
||||
val accuracy: PredictionAccuracy = PredictionAccuracy.Empty,
|
||||
val today: LocalDate = LocalDate.EPOCH,
|
||||
val today: LocalDate = LocalDate.ofEpochDay(0),
|
||||
) {
|
||||
val daysUntil: Long?
|
||||
get() = forecast?.let { it.mostLikelyStartDate.toEpochDay() - today.toEpochDay() }
|
||||
|
|
|
|||
|
|
@ -1,4 +1,11 @@
|
|||
<?xml version="1.0" encoding="utf-8"?>
|
||||
<!--
|
||||
The -v26 qualifier stays despite lint's ObsoleteSdkInt warning, which is
|
||||
technically right (minSdk is 26) and practically wrong: moving these into a
|
||||
plain `mipmap-anydpi` folder makes AAPT fail to resolve mipmap/ic_launcher at
|
||||
all. Tried it; the build stopped with "resource mipmap/ic_launcher not found".
|
||||
A silenced warning is not worth a broken icon.
|
||||
-->
|
||||
<adaptive-icon xmlns:android="http://schemas.android.com/apk/res/android">
|
||||
<background android:drawable="@color/ic_launcher_background" />
|
||||
<foreground android:drawable="@drawable/ic_launcher_foreground" />
|
||||
|
|
|
|||
178
build.gradle.kts
178
build.gradle.kts
|
|
@ -9,4 +9,182 @@ plugins {
|
|||
alias(libs.plugins.kotlin.compose) apply false
|
||||
alias(libs.plugins.ksp) apply false
|
||||
alias(libs.plugins.hilt) apply false
|
||||
alias(libs.plugins.room) apply false
|
||||
}
|
||||
|
||||
// ===========================================================================
|
||||
// Module boundaries
|
||||
// ===========================================================================
|
||||
//
|
||||
// The dependency table in docs/architecture/README.md, as a check.
|
||||
//
|
||||
// Two boundaries in this project are load-bearing and neither can be held up by
|
||||
// people remembering them:
|
||||
//
|
||||
// 1. `domain:*` must not see the Android SDK. It is why the prediction
|
||||
// engine's tests run in a second instead of on an emulator, and one
|
||||
// contributor reaching for a convenient Android API would end that with no
|
||||
// test failing.
|
||||
// 2. The advertising subsystem must never reach cycle data. PRODUCT_PLAN.md
|
||||
// §34 states it as non-negotiable: no menstrual date, cycle length,
|
||||
// fertility status, ovulation estimate, prediction confidence, prediction
|
||||
// history or spotting record may reach advertising, ever.
|
||||
//
|
||||
// The second is written down before `core:ads` exists on purpose. A guard added
|
||||
// alongside the code it constrains is a guard that was shaped around whatever
|
||||
// exception somebody wanted at the time.
|
||||
//
|
||||
// Run: ./gradlew checkModuleBoundaries (also wired into `check`)
|
||||
// Prove it fails: bash scripts/prove-guard.sh
|
||||
//
|
||||
// Exit is non-zero with every violation listed, not just the first — a guard
|
||||
// that reports one problem per run turns a five-minute fix into five runs.
|
||||
|
||||
/** Project dependencies each module is permitted. Anything else fails. */
|
||||
val allowedProjectDependencies: Map<String, Set<String>> = mapOf(
|
||||
":app" to setOf(":core:designsystem", ":core:data", ":core:datastore", ":domain:cycle", ":domain:prediction"),
|
||||
":core:designsystem" to emptySet(),
|
||||
":core:database" to setOf(":domain:cycle", ":domain:prediction"),
|
||||
":core:datastore" to emptySet(),
|
||||
":core:data" to setOf(":core:database", ":domain:cycle", ":domain:prediction"),
|
||||
":domain:cycle" to emptySet(),
|
||||
":domain:prediction" to setOf(":domain:cycle"),
|
||||
// Batch 07. Empty, and that is the whole point: the ads module may reach
|
||||
// NOTHING in this project. It talks to the UI through an AdProvider
|
||||
// interface owned by :app.
|
||||
":core:ads" to emptySet(),
|
||||
)
|
||||
|
||||
/** Modules that must never see the Android SDK, by never applying an Android plugin. */
|
||||
val mustStayPureJvm = setOf(":domain:cycle", ":domain:prediction")
|
||||
|
||||
/**
|
||||
* Configurations that describe what SHIPS. Test-only dependencies are not a
|
||||
* product boundary — a test may reach for a fake or an in-memory database that
|
||||
* production must not — so they are deliberately not examined here.
|
||||
*/
|
||||
val shippingConfigurations = setOf("implementation", "api", "compileOnly", "runtimeOnly", "ksp")
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Collected in afterEvaluate, and that is not a detail.
|
||||
//
|
||||
// The first version of this read `subprojects.configurations` directly in the
|
||||
// root build script. The root project is configured BEFORE its subprojects, so
|
||||
// every configuration was empty, every module had no dependencies, and the task
|
||||
// printed "7 modules checked, no violations" while checking nothing at all.
|
||||
//
|
||||
// It was caught by `scripts/prove-guard.sh` on the first run — a deliberate
|
||||
// forbidden dependency was added to :domain:prediction and the guard stayed
|
||||
// green. That is precisely the failure GUARDS.md §1 exists for, and it is why
|
||||
// no guard here is believed until it has been watched failing.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
val observedProjectDependencies = mutableMapOf<String, Set<String>>()
|
||||
val observedAndroidPlugins = mutableMapOf<String, List<String>>()
|
||||
val containerProjects = mutableSetOf<String>()
|
||||
|
||||
subprojects {
|
||||
afterEvaluate {
|
||||
if (!buildFile.exists()) {
|
||||
// `include(":core:database")` makes Gradle create an intermediate
|
||||
// `:core` project with nothing to build. Containers, not modules —
|
||||
// named in the output rather than dropped, because "skipped" and
|
||||
// "passed" must not look the same.
|
||||
containerProjects += path
|
||||
return@afterEvaluate
|
||||
}
|
||||
observedProjectDependencies[path] = configurations
|
||||
.filter { it.name in shippingConfigurations }
|
||||
.flatMap { conf -> conf.dependencies.filterIsInstance<ProjectDependency>() }
|
||||
.map { it.path }
|
||||
.toSet()
|
||||
observedAndroidPlugins[path] = plugins.mapNotNull { plugin ->
|
||||
plugin::class.qualifiedName?.takeIf { it.contains("com.android.build") }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
tasks.register("checkModuleBoundaries") {
|
||||
group = "verification"
|
||||
description = "Every module's project dependencies must match docs/architecture/README.md."
|
||||
|
||||
val allowed = allowedProjectDependencies
|
||||
val observed = observedProjectDependencies
|
||||
val androidPlugins = observedAndroidPlugins
|
||||
val pureJvm = mustStayPureJvm
|
||||
val containers = containerProjects
|
||||
|
||||
// Providers, not values: these maps are filled during afterEvaluate, which
|
||||
// has not run when this task is being registered. Reading them eagerly here
|
||||
// is the same mistake as reading them in the root script.
|
||||
inputs.property("allowed", allowed.toString())
|
||||
inputs.property("observed", provider { observed.toString() })
|
||||
inputs.property("androidPlugins", provider { androidPlugins.toString() })
|
||||
|
||||
doLast {
|
||||
val violations = mutableListOf<String>()
|
||||
|
||||
val modules = observed.keys
|
||||
|
||||
// Refuse to report a pass over nothing. An empty map here means the
|
||||
// collection above did not run, which is exactly how this guard was
|
||||
// green while checking nothing.
|
||||
if (modules.isEmpty()) {
|
||||
throw GradleException(
|
||||
"no modules were examined, so nothing was checked. This is not a pass — " +
|
||||
"see the afterEvaluate note in build.gradle.kts.",
|
||||
)
|
||||
}
|
||||
|
||||
// A module nobody declared a rule for is not "allowed anything" — it is
|
||||
// unmeasured, and reporting it as a pass is how a boundary quietly stops
|
||||
// covering half the project.
|
||||
(modules - allowed.keys).sorted().forEach {
|
||||
violations += "$it has no entry in allowedProjectDependencies, so its dependencies were never checked."
|
||||
}
|
||||
|
||||
observed.forEach { (module, deps) ->
|
||||
if (module in containers) return@forEach
|
||||
val permitted = allowed[module] ?: return@forEach
|
||||
(deps - permitted).sorted().forEach { dep ->
|
||||
violations += "$module depends on $dep, which the architecture does not permit."
|
||||
}
|
||||
}
|
||||
|
||||
pureJvm.forEach { module ->
|
||||
androidPlugins[module]?.takeIf { it.isNotEmpty() }?.let { plugins ->
|
||||
violations += "$module applies an Android plugin (${plugins.first()}). " +
|
||||
"It must stay pure JVM so the prediction engine is testable without an emulator."
|
||||
}
|
||||
}
|
||||
|
||||
if (violations.isNotEmpty()) {
|
||||
logger.error("")
|
||||
logger.error("Module boundary violations:")
|
||||
violations.forEach { logger.error(" - $it") }
|
||||
logger.error("")
|
||||
logger.error("These are the rules in docs/architecture/README.md. If the architecture")
|
||||
logger.error("changed on purpose, change that table and the map in build.gradle.kts in")
|
||||
logger.error("the same commit. If it did not, this dependency is the mistake.")
|
||||
throw GradleException("${violations.size} module boundary violation(s).")
|
||||
}
|
||||
|
||||
logger.lifecycle(
|
||||
"module boundaries: ${modules.size} module(s) checked, " +
|
||||
"${pureJvm.size} required to stay pure JVM, no violations.",
|
||||
)
|
||||
if (containers.isNotEmpty()) {
|
||||
logger.lifecycle(
|
||||
" (skipped ${containers.size} container project(s) with no build file: " +
|
||||
"${containers.sorted().joinToString(", ")})",
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Wired into `check` so it runs with the tests rather than only when remembered.
|
||||
subprojects {
|
||||
tasks.matching { it.name == "check" }.configureEach {
|
||||
dependsOn(rootProject.tasks.named("checkModuleBoundaries"))
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -247,7 +247,9 @@ class CycleRepository internal constructor(
|
|||
*/
|
||||
private suspend fun scoreOutstanding(actualStart: LocalDate) {
|
||||
val standing = predictionDao.unscored().firstOrNull() ?: return
|
||||
val madeOn = LocalDate.ofInstant(standing.generatedAt, clock.zone)
|
||||
// NOT LocalDate.ofInstant — that is API 34, and minSdk here is 26.
|
||||
// Lint caught it; an API 36 emulator never would have.
|
||||
val madeOn = standing.generatedAt.atZone(clock.zone).toLocalDate()
|
||||
if (madeOn.isAfter(actualStart)) return // backfill
|
||||
|
||||
val error = abs(standing.predictedStartDate.toEpochDay() - actualStart.toEpochDay()).toInt()
|
||||
|
|
@ -266,7 +268,8 @@ class CycleRepository internal constructor(
|
|||
*/
|
||||
private suspend fun snapshotForecast(basedOnPeriodId: Long?) {
|
||||
val starts = periodDao.confirmedStartDates()
|
||||
val notYet = notYetDao.since(LocalDate.EPOCH).map { it.toDomain() }
|
||||
// LocalDate.EPOCH is API 34. ofEpochDay(0) is the same date and is API 26.
|
||||
val notYet = notYetDao.since(LocalDate.ofEpochDay(0)).map { it.toDomain() }
|
||||
val prediction = engine.predict(starts, today(), notYet) ?: return
|
||||
predictionDao.deleteUnscored()
|
||||
predictionDao.insert(prediction.toEntity(clock.instant(), basedOnPeriodId))
|
||||
|
|
|
|||
|
|
@ -141,9 +141,48 @@ exists it will declare no dependency on core/database or `domain/*`, and a
|
|||
Gradle check enforces the whole table above by enumerating each module's allowed
|
||||
dependencies. Ads reach the UI through an `AdProvider` interface owned by `app`.
|
||||
|
||||
Per [`GUARDS.md`](GUARDS.md) §1, that check is proved to fail — a deliberate
|
||||
forbidden dependency added, the guard watched going red, the file restored —
|
||||
before it is treated as evidence. `scripts/prove-guard.sh` performs it.
|
||||
### The guard, and the three proofs it survived
|
||||
|
||||
`checkModuleBoundaries` in the root `build.gradle.kts` holds the tables above as
|
||||
a check. It runs as part of `./gradlew check`, lists **every** violation rather
|
||||
than the first, and refuses to report a pass when it examined no modules at all.
|
||||
|
||||
Per [`GUARDS.md`](GUARDS.md) §1 it is not evidence until it has been watched
|
||||
failing. These three are repeatable, each restores the file from a `trap`, and
|
||||
each was run:
|
||||
|
||||
```bash
|
||||
# 1. A domain module reaching upward — the leak that would end JVM-only tests
|
||||
bash scripts/prove-guard.sh domain/prediction/build.gradle.kts \
|
||||
'implementation(project(":domain:cycle"))' \
|
||||
'implementation(project(":domain:cycle"))
|
||||
implementation(project(":core:database"))' \
|
||||
./gradlew checkModuleBoundaries
|
||||
|
||||
# 2. :app reaching past the repository straight to Room
|
||||
bash scripts/prove-guard.sh app/build.gradle.kts \
|
||||
'implementation(project(":core:data"))' \
|
||||
'implementation(project(":core:data"))
|
||||
implementation(project(":core:database"))' \
|
||||
./gradlew checkModuleBoundaries
|
||||
|
||||
# 3. A module with no rule is reported as unmeasured, not assumed fine
|
||||
bash scripts/prove-guard.sh build.gradle.kts \
|
||||
'":core:data" to setOf(":core:database", ":domain:cycle", ":domain:prediction"),' \
|
||||
'' \
|
||||
./gradlew checkModuleBoundaries
|
||||
```
|
||||
|
||||
**Proof 1 failed the first time, and that is the point.** The guard reported
|
||||
*"7 modules checked, no violations"* with a forbidden dependency sitting in the
|
||||
build file. The root project is configured before its subprojects, so reading
|
||||
`subprojects.configurations` from the root script saw every configuration empty
|
||||
— the check had been green over an empty map since the moment it was written.
|
||||
Collection now happens in `afterEvaluate`, and the task throws rather than
|
||||
passing if it ends up with no modules to examine.
|
||||
|
||||
Thirty seconds of proving caught a guard that would otherwise have been trusted
|
||||
for months.
|
||||
|
||||
## Data shapes
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue