| name | architecture-review |
| description | Review a code change against Code On The Go's architecture — ARCHITECTURE.md and the ADRs in docs/adr/. Forces a read of those authoritative docs (they are NOT reliably in context otherwise), then checks the diff against UDF/state, Koin DI, Room-vs-SQLite, Compose, module boundaries, ABI flavors, dependency substitution, @Parcelize, and strings placement, reporting violations with the ADR/section each comes from. Use when asked to review architecture alignment, check a change against ARCHITECTURE.md/the ADRs, or as the §10 step of a code review. |
| metadata | {"author":"Hal Eisen","keywords":["architecture","review","adr","udf","code-review","codeonthego"]} |
Why this is a skill (not just REVIEW.md §10)
ARCHITECTURE.md (~4.9k tokens) and the ADRs (~7.3k) are not in context during normal work, and prose links in REVIEW.md are not reliably followed. This skill exists to guarantee the read: it opens the authoritative docs, then checks the diff against them. Do not review architecture from memory or from the summary in this file alone.
When to invoke
- "Review this against the architecture / ARCHITECTURE.md / the ADRs."
- "Does this change follow our patterns?" / architecture-alignment review.
- As the §10 step of a full code review — its output feeds REVIEW.md's evidence ledger.
Step 1 — Scope the diff
Pick the target and get the changed files + hunks:
- Working tree:
git diff --stat and git diff.
- This branch vs the integration branch:
git diff origin/stage...HEAD (feature branches are based on stage).
- A GitHub PR:
gh pr diff <N>.
Exclude vendored/generated code — it's not held to our patterns: composite-builds/build-deps*, subprojects/{aaptcompiler,builder-model-impl,flashbar,xml-dom}, termux/, eventbus/, LayoutEditor/, **/build/, generated R/BuildConfig. Review only first-party Kotlin/Java/XML/Gradle changes.
Step 2 — READ the authoritative docs (mandatory)
Before judging anything, read:
ARCHITECTURE.md (whole file — module map, dependency rules, tech stack, State Management, testing).- Every file in
docs/adr/*.md. At minimum open the ones a diff can plausibly violate: 0001 (Room), 0003 (substitution), 0005 (flavors), 0006 (Koin), 0009 (Compose). Read the rest if the change is broad.
Do not skip this because the rules "look familiar" — they are the source of truth and they change.
Step 3 — Check the diff against the rules
For each changed first-party file, check the applicable rules. Each rule cites its source so findings are traceable.
| # | Rule | Source |
|---|
| 1 | UDF: new screens use ViewModel + StateFlow<UiState>, sealed UiEvent/UiEffect, a repository for data; composables collect via collectAsStateWithLifecycle() (lifecycle-aware, Android's strongly-recommended default; collectAsState() is only for platform-agnostic/KMP code, which we don't have); no I/O or business logic in composables/Activities/Fragments. | ARCHITECTURE.md → State Management |
| 2 | Sealed state for mutually-exclusive states (loading/content/error/…): not a data class of independent Booleans that can contradict each other ("boolean hell"). | ARCHITECTURE.md → State Management |
| 3 | Koin DI, constructor injection; register new singletons/ViewModels in the module. No hand-rolled singletons/service locators (the documented ServiceLocator aside). | ADR 0006 |
| 4 | Persistence: Room is the default for relational data; raw SQLite only for a justified exception (prebuilt read-only DB, perf/allocation-critical indexing, cross-boundary schema) — and the PR must say which. Non-relational → filesystem/preferences (DataStore). | ADR 0001 |
| 5 | @Parcelize (kotlin-parcelize) for Parcelable; never hand-implement it unless Parcelize genuinely can't. | ARCHITECTURE.md → Parceling |
| 6 | New UI is Jetpack Compose — a new XML-layout / Fragment-rendered screen for the IDE's own UI is a violation (existing XML screens are fine until reworked). | ADR 0009 |
| 7 | Module boundaries / dependency direction: UI → ViewModel → Repository → data source; features depend on common/utils, not the reverse; no new cross-feature or upward dependency. | ARCHITECTURE.md → module map |
| 8 | ABI flavors: new Android modules get v7/v8 via composite-builds/build-logic centrally — no per-module flavor blocks, no flavorless . ( is intentionally flavorless.) |
Rules 1, 2, 6, 10, 11, 12 apply to UI changes; 4, 5 to data/model changes; 8, 9 to Gradle changes; 3 wherever a dependency, singleton, or ViewModel is introduced; 7 wherever a module's dependencies or cross-module imports change. Any rule not listed here is still checked whenever the diff touches its subject. Judge by what the diff touches — don't flag rules a file doesn't engage.
For a large diff (~15+ first-party files), fan out: spawn a subagent per dimension (UI/state, DI, persistence, Gradle/modules), each instructed to read the relevant ADR and report only its dimension's findings; then merge. For a small diff, do it inline.
Step 4 — Report
Output a findings table, most-severe first. Every finding must cite the rule source and give a concrete fix. If a rule was checked and passes, say so (the evidence ledger wants pass/fail, not silence).
### Architecture review — <scope>
Docs read: ARCHITECTURE.md, docs/adr/0001,0003,0005,0006,0009 (+others as needed)
| Verdict | File:line | Rule | Source | Fix |
|---|---|---|---|---|
| ❌ | ui/FooScreen.kt:42 | Mutually-exclusive state as booleans | ADR 0001 sibling / State Mgmt | Model as a sealed FooUiState (Loading/Content/Error) |
| ⚠️ | data/BarStore.kt:10 | Raw SQLite without stated justification | ADR 0001 | Use Room, or state which exception applies in the PR |
| ✅ | — | Koin DI | ADR 0006 | new VM registered in module, constructor-injected |
Summary: <n> violations, <n> warnings, <n> checks passed.
- ❌ violation = contradicts a rule; blocking. ⚠️ warning = likely issue / missing justification. ✅ = checked and clean.
- If nothing architectural changed (e.g. a docs- or test-only diff), say so explicitly rather than inventing findings.
Notes
- This is a conformance check against our documented patterns — not a general bug hunt (use
/code-review for correctness). Keep findings tied to a specific ADR/section.
- If a rule seems wrong or outdated for the change at hand, flag it as a possible ADR update rather than forcing the code to fit — the ADRs are
Proposed, not immutable.
