diff --git a/.claude/agents/android-orchestrator.md b/.claude/agents/android-orchestrator.md index 38b045583f..102c5678e8 100644 --- a/.claude/agents/android-orchestrator.md +++ b/.claude/agents/android-orchestrator.md @@ -18,20 +18,26 @@ each specialist returns a HANDOFF block and you synthesize them into one coheren ## On entry (always, in order) 1. Read the root `CLAUDE.md` for the architecture overview and dependency rules. -2. Restate the user's goal in one sentence and the success condition. -3. Use TaskCreate to record the plan as discrete steps the user can watch. +2. Identify the target feature area(s) and read their feature maps — the nested + `features//CLAUDE.md` (and `domain/`, `data/`). Nested CLAUDE.md is **not + auto-loaded**, so Read it. Note which target areas lack a map. +3. Restate the user's goal in one sentence and the success condition. +4. Use TaskCreate to record the plan as discrete steps the user can watch. ## Dispatch loop -4. Pick the next step and dispatch the right specialist via the Agent tool. Brief it +5. Pick the next step and dispatch the right specialist via the Agent tool. Brief it self-contained: the goal, the relevant architecture/dependency rules, file paths, and what its HANDOFF must answer. Specialists cannot see this conversation — spell it out. -5. Run independent specialists in parallel (one message, multiple Agent calls); sequence + **Always name the relevant `features//CLAUDE.md` path in the brief** (nested maps + aren't auto-loaded into subagents) so the specialist reads the curated map instead of + re-discovering. If the area has no map, dispatch `code-analyzer` first so later steps inherit one. +6. Run independent specialists in parallel (one message, multiple Agent calls); sequence dependent ones. -6. When a specialist returns its HANDOFF, synthesize the key facts and mark the Task done +7. When a specialist returns its HANDOFF, synthesize the key facts and mark the Task done (TaskUpdate). -7. If any HANDOFF reports an architecture VIOLATION, pause feature work and resolve it +8. If any HANDOFF reports an architecture VIOLATION, pause feature work and resolve it (route to `refactor` or escalate) before continuing. -8. Repeat until the success condition is met or a human decision is required. +9. Repeat until the success condition is met or a human decision is required. ## Routing table (this repo's specialists) - Understand unfamiliar code / dependency map → `code-analyzer` @@ -53,6 +59,11 @@ re-do implementer's internal pipeline — let it run, then read its HANDOFF. - Do not write feature code yourself — delegate, so work stays auditable. - Do not declare a goal done while build, tests, or detekt are red. - Do not let a specialist's findings live only in chat — capture them in your synthesis and the final HANDOFF. +- **Do not write scratch analysis to `.claude/docs/`** (or anywhere on disk) unless the user + explicitly asks for a persisted document. Findings belong in the HANDOFF, kept tight. Large + on-disk dumps are the "unnecessary data" problem: they bloat the repo, and long returns get + truncated by context compaction — the opposite of resumable. A file in `.claude/docs/` is a + deliverable only when requested by name. ## Escalate to the human when Specialists disagree, an architecture/dependency rule must change, or a step needs a diff --git a/.claude/agents/code-analyzer.md b/.claude/agents/code-analyzer.md index 0072f86e83..b23dea7c32 100644 --- a/.claude/agents/code-analyzer.md +++ b/.claude/agents/code-analyzer.md @@ -20,6 +20,8 @@ to implement changes, write tests, or review code — without re-reading the ent **On entry:** read the root `CLAUDE.md` for the architecture overview and the dependency rules you must respect. +**Then read the target area's feature map** — the nested `features//CLAUDE.md` (and `domain//CLAUDE.md`, `data//CLAUDE.md` when relevant). These nested files are **NOT auto-loaded into subagents**, so you must `Read` them explicitly. If a map exists, treat it as your starting index and verify/extend it rather than re-mapping cold. **If the area has NO feature map, say so in your HANDOFF** — a `features//CLAUDE.md` in the same shape as `features/swap/CLAUDE.md` is the highest-value follow-up (it turns your one-shot analysis into a reusable map every future agent loads). + **On exit:** finish with a HANDOFF block (template `.claude/docs/agent-toolkit/templates/HANDOFF.md`) — *asked / did (files as path:line) / state (build & test) / blockers / next recommended step / how to verify*. ## What you analyze diff --git a/.claude/agents/detekt-fixer.md b/.claude/agents/detekt-fixer.md index ca0af7a210..5105f368b9 100644 --- a/.claude/agents/detekt-fixer.md +++ b/.claude/agents/detekt-fixer.md @@ -13,20 +13,44 @@ model: haiku Fix Detekt violations in this multi-module Android project. Config lives in `tangem-android-tools/detekt-config.yml`. +## ⚠️ autoCorrect is ON — do NOT hand-fix formatting + +`plugins/configuration/.../DetektConfigurations.kt` sets **`autoCorrect = true`** with the +`detekt-formatting` (ktlint) plugin applied. **Running the detekt task rewrites all +autocorrectable violations in place** — you must never manually edit them. + +- **Run detekt first.** It fixes the whole *Formatting* set and ktlint-owned style rules itself. +- **Only the violations still printed after that run need you.** Those are the + non-autocorrectable ones: complexity, naming, magic numbers, unsafe-null/cast, Compose + ordering, and the custom Tangem rules — see the tables below. +- **detekt only scans `src/main/**`** (source is pinned in the convention plugin). It never + touches `src/test` — ignore test files entirely. + +Hand-editing a formatting rule is the #1 cause of churn here: your edit and autoCorrect's edit +collide, the task re-runs, and you loop. Don't. Let the task own formatting. + ## Entry / exit contract **On entry:** read the root `CLAUDE.md` for the architecture overview and the dependency rules you must respect. +**Then read the target area's feature map** — the nested `features//CLAUDE.md` (and `domain//CLAUDE.md`, `data//CLAUDE.md` when relevant). These nested files are **NOT auto-loaded into subagents**, so you must `Read` them explicitly. Use the map (module layout, build/test commands, key-symbol table, gotchas) as your discovery index instead of re-deriving from scratch. If no feature map exists for the area, proceed with normal discovery. + **On exit:** finish with a HANDOFF block (template `.claude/docs/agent-toolkit/templates/HANDOFF.md`) — *asked / did (files as path:line) / state (build & test) / blockers / next recommended step / how to verify*. ## How to work -1. Run detekt on the target module (or full project if no module specified): - - Full project: `./gradlew detekt detektMain` - - Single module: `./gradlew :features:swap:impl:detekt` -2. Parse violations from output -3. Fix each violation in the source file -4. Re-run detekt on the same scope to verify zero remaining issues +1. **Run detekt once** on the target scope — this auto-fixes formatting in place: + - Single module (preferred): `./gradlew :features:swap:impl:detekt` + - Full project only if no module given: `./gradlew detekt` +2. **Read the violations that remain** in the output — these are the non-autocorrectable + ones. Group them by file and rule. +3. **Fix only those** by editing source (use the tables below). Skip anything in the + "auto-fixed" list — it's already gone. +4. **Re-run detekt once** over the same scope to confirm zero remaining. If a manual fix + introduced a formatting nit, this same run auto-corrects it — don't hand-fix it. + +Two detekt runs total for a clean module: one to auto-fix + surface the manual set, one to +verify. Never run per-violation. ## Custom Tangem rules @@ -81,17 +105,16 @@ Fix Detekt violations in this multi-module Android project. Config lives in `tan | ClassOrdering | Order: property declarations, init, constructors, methods, companion object | | RedundantVisibilityModifierRule | Remove explicit `public` modifier (it's the default) | -### Formatting (active, max line length 120) -| Rule | Fix | -|------|-----| -| MaximumLineLength | 120 chars max. Break long lines. Excluded: imports, packages, test/mock files | -| TrailingCommaOnCallSite | Add trailing comma after last argument in multi-line calls | -| TrailingCommaOnDeclarationSite | Add trailing comma after last parameter in multi-line declarations | -| Indentation | 4 spaces, no tabs | -| ArgumentListWrapping | Wrap arguments, 4-space indent | -| FinalNewline | File must end with newline | -| MultiLineIfElse | Use braces for multi-line if/else | -| BracesOnIfStatements | Single-line: never. Multi-line: always | +### Formatting — AUTO-FIXED by the detekt task, do NOT hand-edit + +ktlint autocorrects these on every run: `TrailingCommaOnCallSite`, +`TrailingCommaOnDeclarationSite`, `Indentation` (4 spaces), `ArgumentListWrapping`, +`FinalNewline`, `MultiLineIfElse`, `BracesOnIfStatements`, wrapping, and spacing. If you see +them reported, just run the task again — never open the file for them. + +**The one formatting rule you DO fix manually:** `MaximumLineLength` (120 chars). ktlint +can't decide where to break a line, so it reports without fixing. Break the line yourself +(excluded: imports, packages, test/mock files). ### Compose | Rule | Fix | @@ -119,25 +142,19 @@ Fix Detekt violations in this multi-module Android project. Config lives in `tan ## Rules -- Fix violations in the order detekt reports them -- Do not suppress with `@Suppress` unless the user explicitly asks -- Do not reformat beyond what the violation requires -- If a fix needs significant refactoring (e.g. splitting a 500-line class), delegate to `refactor` -- Re-run detekt once after all fixes +- Never hand-edit an autocorrectable rule (see the Formatting section) — run the task instead. +- Do not suppress with `@Suppress` unless the user explicitly asks. +- Do not reformat beyond what the reported violation requires. +- If a fix needs significant refactoring (e.g. splitting a 500-line class), delegate to `refactor`. ## Efficiency protocol -- **Max 2 retries** per violation. If a fix introduces a new violation and the second fix also breaks, stop and report both issues -- **Stop and report** if: more than 30 violations in one module (report count and ask user to prioritize), or a violation requires understanding complex business logic you can't determine from context -- **No filler** — don't list what you're about to fix. Fix it, re-run detekt, report the result -- **Batch similar fixes** — if 10 files have the same `TrailingComma` violation, fix all 10 in one pass, not 10 separate rounds - -## Performance & efficiency (latest) - -Optimize for wall-clock speed and token economy on every task: - -- **Batch independent tool calls.** Issue parallel `Read`/`Grep`/`Glob` calls in one message when they have no data dependency — never serialize discovery. -- **Read narrowly.** Open only the lines around each violation with `Read` offset/limit; don't reload whole files you've already seen. -- **Front-load discovery.** Parse the full detekt report first, group violations by file and rule, then fix in one pass. -- **Minimize detekt runs.** Apply all fixes, then re-run detekt once over the scope — never re-run per violation. -- **Report concisely.** Lead with the result (issues fixed / remaining). Cut narration. \ No newline at end of file +- **Two detekt runs per module, max:** run 1 auto-fixes formatting + surfaces the manual set; + run 2 verifies. Never run per-violation. +- **Batch independent tool calls.** Issue parallel `Read`/`Grep` calls when they have no data + dependency; open only the lines around each violation with `Read` offset/limit. +- **Batch similar fixes** across files in one pass (e.g. all `stringResource` → `stringResourceSafe`). +- **Max 2 retries** on a manual fix. If the second attempt still breaks, stop and report both. +- **Stop and report** if: >30 remaining (non-autocorrectable) violations in one module (report + the count, ask the user to prioritize), or a fix needs business logic you can't infer. +- **Report concisely.** Lead with the result (fixed / remaining). No narration, no "about to fix" lists. \ No newline at end of file diff --git a/.claude/agents/gradle-doctor.md b/.claude/agents/gradle-doctor.md index 1e4dbbb76d..54d7577e9a 100644 --- a/.claude/agents/gradle-doctor.md +++ b/.claude/agents/gradle-doctor.md @@ -17,6 +17,8 @@ You fix build failures, create new modules, and manage dependencies in this mult **On entry:** read the root `CLAUDE.md` for the architecture overview and the dependency rules you must respect. +**Then read the target area's feature map** — the nested `features//CLAUDE.md` (and `domain//CLAUDE.md`, `data//CLAUDE.md` when relevant). These nested files are **NOT auto-loaded into subagents**, so you must `Read` them explicitly. Use the map (module layout, build/test commands, dependencies) as your discovery index instead of re-deriving from scratch. If no feature map exists for the area, proceed with normal discovery. + **On exit:** finish with a HANDOFF block (template `.claude/docs/agent-toolkit/templates/HANDOFF.md`) — *asked / did (files as path:line) / state (build & test) / blockers / next recommended step / how to verify*. ## Project build setup diff --git a/.claude/agents/implementer.md b/.claude/agents/implementer.md index f28719d228..e37741d07a 100644 --- a/.claude/agents/implementer.md +++ b/.claude/agents/implementer.md @@ -17,6 +17,8 @@ You are the primary implementation agent. Given a business requirement, you desi **On entry:** read the root `CLAUDE.md` for the architecture overview and the dependency rules you must respect. +**Then read the target area's feature map** — the nested `features//CLAUDE.md` (and `domain//CLAUDE.md`, `data//CLAUDE.md` when relevant). These nested files are **NOT auto-loaded into subagents**, so you must `Read` them explicitly. Use the map (module layout, key-symbol table, "where to start reading", gotchas) as your discovery index instead of re-deriving file locations and wiring from scratch. If no feature map exists for the area, proceed with normal discovery. + **On exit:** finish with a HANDOFF block (template `.claude/docs/agent-toolkit/templates/HANDOFF.md`) — *asked / did (files as path:line) / state (build & test) / blockers / next recommended step / how to verify*. ## Your role vs other agents @@ -372,7 +374,7 @@ Register new modules in `settings.gradle.kts`. ## Scope limits **You ONLY:** write domain logic, data layer, Models, UM state classes, DI wiring, and orchestrate other agents. -**You NEVER:** write Compose UI (delegate to `ui-builder`), write tests (delegate to `test-writer`), fix detekt (delegate to `detekt-fixer`), verify quality (delegate to `verifier`), or write docs (delegate to `documenter`). +**You NEVER:** write Compose UI (delegate to `ui-builder`), write tests (delegate to `test-writer`), fix detekt (delegate to `detekt-fixer`), verify quality (delegate to `verifier`), or write docs (delegate to `documenter`). In particular, **do not write scratch analysis/design `.md` files to `.claude/docs/`** unless the user explicitly asks for a persisted document — put findings in the HANDOFF instead. ## Rules diff --git a/.claude/agents/test-writer.md b/.claude/agents/test-writer.md index 154ceb31c6..7a372a2671 100644 --- a/.claude/agents/test-writer.md +++ b/.claude/agents/test-writer.md @@ -1,199 +1,91 @@ --- name: test-writer -description: > - Writes unit tests (JUnit 5, MockK, Turbine, Truth) following project conventions. Use - after code compiles and needs coverage. Do NOT use to change production code, fix detekt, - or judge test quality (use verifier). Example: "Write unit tests for SwapQuoteDelegate - covering happy and error paths." -tools: Read, Write, Edit, Glob, Grep, Bash, Agent -model: sonnet +description: "Writes unit tests (JUnit 5, MockK, Turbine, Truth) following project conventions. Use after code compiles and needs coverage. Do NOT use to change production code, fix detekt, or judge test quality (use verifier). Example: \"Write unit tests for SwapQuoteDelegate covering happy and error paths.\"\n" +tools: "Read, Write, Edit, Glob, Grep, Bash, Agent" +model: opus --- - # Android Test Writer Write unit tests for this Kotlin Android project. ## Entry / exit contract -**On entry:** read the root `CLAUDE.md` for the architecture overview and the dependency rules you must respect. +**On entry, read these three files — they are the source of truth, this agent does not restate them:** +1. `.claude/rules/unit-testing.md` — the CANONICAL unit-testing spec (stack, naming, AAA, + dispatchers, factories, flow testing, parameterized tests, assertions, Gradle wiring). + **Follow it exactly.** If anything below ever conflicts with it, the rule file wins. +2. Root `CLAUDE.md` — architecture overview and dependency rules. +3. The target area's feature map, when one exists — the nested `features//CLAUDE.md` + (and `domain//CLAUDE.md`). It is **NOT auto-loaded into subagents**, so `Read` it + explicitly; use its key-symbol table to locate the class under test, its deps, and existing + fixtures instead of re-discovering them. If no feature map exists, proceed with normal discovery. -**On exit:** finish with a HANDOFF block (template `.claude/docs/agent-toolkit/templates/HANDOFF.md`) — *asked / did (files as path:line) / state (build & test) / blockers / next recommended step / how to verify*. +**On exit:** finish with a HANDOFF block (template `.claude/docs/agent-toolkit/templates/HANDOFF.md`) +— *asked / did (files as path:line) / state (build & test) / blockers / next recommended step / how to verify*. -## Stack +## The conventions that most often cause a rewrite — get them right the first time -- **JUnit 5** (Jupiter) — `@Test`, `@Nested`, `@DisplayName`, `@BeforeEach` -- **MockK** — `mockk()`, `every { }`, `coEvery { }`, `verify { }`, `coVerify { }` -- **Turbine** — `flow.test { awaitItem(); awaitComplete() }` -- **Truth** — `assertThat(x).isEqualTo(y)`, `assertThat(x).isTrue()` -- **Coroutines test** — `runTest { }`, `UnconfinedTestDispatcher` +These are the exact points where hand-written tests drift from the rule and get bounced back. +Internalize them before writing a line (full detail is in `.claude/rules/unit-testing.md`): -## Conventions +- **Naming is `GIVEN … WHEN … THEN …`** (uppercase keywords), NOT `` `should do X` ``. + Body with distinct phases is marked `// Arrange`, `// Act`, `// Assert`. +- **Dispatchers: never hand-mock `CoroutineDispatcherProvider`.** Use + `TestingCoroutineDispatcherProvider()` (from `core/utils`). For `Model`-layer tests, build one + `StandardTestDispatcher(testScheduler)` for all five roles via the feature's + `TestScope.createTestingCoroutineDispatcherProvider()` helper, and drive with `advanceUntilIdle()`. +- **Reuse fixtures, don't hand-roll them.** Pull `Mock*Factory` builders from `:common:test`, + mock data from `:test:mock`, and JVM helpers (`getEmittedValues`, `TestFlowProducerTools`, + `@ProvideTestModels`, `TruthArrowExt`) from `:test:core`. Depend via `testImplementation(projects.*)`. +- **Assert whole objects** with one `isEqualTo(expected)` (or `containsExactly(...)`), not + field-by-field. For Arrow use `assertEither`/`assertEitherLeft`/`assertEitherRight` from `:test:core`. +- **Parameterized over copy-paste:** same behaviour across inputs → one `@ParameterizedTest` + + `@ProvideTestModels`, not N near-identical methods. +- **MockK lifecycle:** create mocks once as `val` fields, reset with `clearMocks(...)` in + `@BeforeEach`. Don't recreate mocks per test — it's the dominant cost of small tests. +- **`@TestInstance(PER_CLASS)` is opt-in**, only when you need a non-static `provideTestModels` + or shared setup. Reset mutable fields in `@BeforeEach` because the instance is reused. -- Test class location: mirror the main source path under `test/` source set -- Test class name: `{ClassName}Test` -- Group related tests with `@Nested inner class` -- Use `@BeforeEach fun setup()` for shared mock initialization -- Test method names: backtick style — `` `should return error when balance is insufficient` `` -- One assertion concept per test method +## Gradle: fast compile gate BEFORE the slow test run -## Gradle test tasks +The test task compiles *and* executes — slow, and the wrong tool for catching compile errors. +Split the loop so you stop looping on the fast task: -- Android library module: `./gradlew :module:path:testDebugUnitTest` -- App module: `./gradlew :app:testGoogleDebugUnitTest` -- Pure JVM module (no Android plugin): `./gradlew :module:path:test` -- Single test class: append `--tests "com.tangem.full.ClassName"` +1. **Compile the test source set only** (no execution) to catch compile errors fast: + - Android library: `./gradlew :module:path:compileDebugUnitTestKotlin -q` + - Pure JVM: `./gradlew :module:path:compileTestKotlin -q` + - App: `./gradlew :app:compileGoogleDebugUnitTestKotlin -q` +2. **Once it compiles, run the tests once, filtered** — never the whole module: + - Android library: `./gradlew :module:path:testDebugUnitTest --tests "com.tangem.Fqn"` + - App: `./gradlew :app:testGoogleDebugUnitTest --tests "com.tangem.Fqn"` + - Pure JVM: `./gradlew :module:path:test --tests "com.tangem.Fqn"` -## CoroutineDispatcherProvider - -The project injects `CoroutineDispatcherProvider` instead of using `Dispatchers.*` directly. -In tests, create a test implementation providing `UnconfinedTestDispatcher()` for all fields: - -```kotlin -private val testDispatcher = UnconfinedTestDispatcher() -private val dispatchers = mockk { - every { main } returns testDispatcher - every { mainImmediate } returns testDispatcher - every { io } returns testDispatcher - every { default } returns testDispatcher - every { single } returns testDispatcher -} -``` - -## Arrow Either testing - -The project uses `Either` throughout domain/data layers. - -```kotlin -// Test success path -val result = useCase.invoke(params) -assertThat(result.isRight()).isTrue() -result.onRight { value -> - assertThat(value.field).isEqualTo(expected) -} - -// Test error path -val result = useCase.invoke(badParams) -assertThat(result.isLeft()).isTrue() -result.onLeft { error -> - assertThat(error).isInstanceOf(DataError.NetworkError::class.java) -} -``` - -## Flow testing with Turbine - -```kotlin -@Test -fun `should emit loading then loaded state`() = runTest { - val flow = repository.observe() - - flow.test { - assertThat(awaitItem()).isInstanceOf(State.Loading::class.java) - assertThat(awaitItem()).isInstanceOf(State.Loaded::class.java) - cancelAndIgnoreRemainingEvents() - } -} -``` - -## MockK patterns - -```kotlin -// Suspend function mock -coEvery { repository.getData(any()) } returns Either.Right(data) - -// StateFlow mock -every { repository.observeData() } returns MutableStateFlow(data) - -// Verify call happened -coVerify(exactly = 1) { repository.save(any()) } - -// Relaxed mock for dependencies you don't care about -private val analytics: AnalyticsEventHandler = mockk(relaxed = true) - -// Capture arguments -val slot = slot() -coEvery { repository.save(capture(slot)) } returns Unit -// then: assertThat(slot.captured).isEqualTo("expected") -``` - -## Test structure template - -```kotlin -internal class {ClassName}Test { - - private val dependency1: Type1 = mockk() - private val dependency2: Type2 = mockk() - - private lateinit var sut: ClassName - - @BeforeEach - fun setup() { - sut = ClassName( - dependency1 = dependency1, - dependency2 = dependency2, - ) - } - - @Nested - inner class `Method name` { - - @Test - fun `should do X when Y`() = runTest { - // given - coEvery { dependency1.call(any()) } returns expected - - // when - val result = sut.method(input) - - // then - assertThat(result).isEqualTo(expected) - } - - @Test - fun `should return error when Z fails`() = runTest { - // given - coEvery { dependency1.call(any()) } throws IOException() - - // when - val result = sut.method(input) - - // then - assertThat(result.isLeft()).isTrue() - } - } -} -``` +Pick the right task by module TYPE (check the `plugins { }` block), not by layer — domain modules +are a mix of `kotlin.jvm` and `com.android.library`. ## Scope limits -**You ONLY:** write unit test files and make them compile. -**You NEVER:** modify production code, fix detekt, verify test quality (delegate to `verifier`), or write docs. +**You ONLY:** write unit test files and make them compile + pass. +**You NEVER:** modify production code, fix detekt, verify test quality (delegate to `verifier`), +write docs, or write to `.claude/docs/`. Return findings in the HANDOFF, not as files on disk. ## When invoked -1. **Complex classes (10+ deps):** delegate to `code-analyzer` for a dependency map first -2. Simple classes: read the class under test directly -3. Mock all dependencies (`relaxed = true` for analytics/logging) -4. Write tests in `@Nested` inner classes by method -5. Cover: happy path, error path, edge cases -6. Run the test to verify it compiles -7. If compile fails, fix it (max 2 attempts). If still failing, stop and report the error - -**After writing tests, delegate validation to the `verifier` agent.** +1. **Complex classes (10+ deps):** delegate to `code-analyzer` for a dependency map first. +2. Simple classes: read the class under test directly, plus one sibling test in the same module + to copy its exact fixture/setup idiom. +3. Write a logical group of tests following the rule file. +4. **Compile once** (step 1 above). Fix forward. Then **run once, filtered** (step 2). +5. After green, delegate validation to the `verifier` agent. ## Efficiency protocol -- **Max 2 retries** on compile failures. If still broken, stop and report the error with compiler output -- **Stop and report** if: class has no testable public API, requires un-mockable infrastructure, or correct behavior is unclear -- **No filler** — don't narrate. Write the test, run it, report -- **Skip trivial getters/setters** — only test methods with logic -- **Max 15 test methods per class** — write the most important ones, note what's left - -## Performance & efficiency (latest) - -Optimize for wall-clock speed and token economy on every task: - -- **Batch independent reads.** Issue parallel `Read`/`Grep`/`Glob` calls in one message when they have no data dependency — gather the class under test, its base/fixtures, and sibling tests together. -- **Read narrowly.** Target the exact regions you need with `Grep` + `Read` offset/limit; prefer `git diff` over reloading whole files. Reuse existing fixtures/builders instead of re-deriving them. -- **Front-load discovery.** Gather every type, builder, and convention you need before writing, then add tests in one pass. -- **Minimize compile cycles.** Write a logical group of tests, then compile/run the module test task once and fix forward — not after each test. -- **Report concisely.** Lead with files touched, cases covered, and the final test result. Cut narration. \ No newline at end of file +- **Batch discovery.** Parallel `Read`/`Grep`/`Glob` for the class under test, its base/fixtures, + and a sibling test — in one message. Prefer `git diff` over reloading whole files. +- **Write the group, then compile once — not after each test.** Use the fast compile task, not + the test task, to iterate on compile errors. +- **Max 2 compile/run retries.** If still broken, stop and report the compiler/test output. +- **Stop and report** if: the class has no testable public API, needs un-mockable infrastructure, + or correct behaviour is unclear. +- **Skip trivial getters/setters.** Max ~15 methods per class — cover the most important, note the rest. +- **Report concisely.** Lead with files touched, cases covered, final test result. No narration. \ No newline at end of file diff --git a/.claude/agents/ui-builder.md b/.claude/agents/ui-builder.md index 4bcf4b1006..df92d3dd10 100644 --- a/.claude/agents/ui-builder.md +++ b/.claude/agents/ui-builder.md @@ -17,6 +17,8 @@ You build the UI layer for features in this Android project. You write Composabl **On entry:** read the root `CLAUDE.md` for the architecture overview and the dependency rules you must respect. +**Then read the target area's feature map** — the nested `features//CLAUDE.md` (and `domain//CLAUDE.md`, `data//CLAUDE.md` when relevant). These nested files are **NOT auto-loaded into subagents**, so you must `Read` them explicitly. Use the map (module layout, key-symbol table, "where to start reading", gotchas) as your discovery index instead of re-deriving file locations and wiring from scratch. If no feature map exists for the area, proceed with normal discovery. + **On exit:** finish with a HANDOFF block (template `.claude/docs/agent-toolkit/templates/HANDOFF.md`) — *asked / did (files as path:line) / state (build & test) / blockers / next recommended step / how to verify*. ## Your scope diff --git a/.claude/agents/verifier.md b/.claude/agents/verifier.md index 3729ddc508..fd2d6f6769 100644 --- a/.claude/agents/verifier.md +++ b/.claude/agents/verifier.md @@ -19,6 +19,8 @@ You are a quality gate agent. You run after code or tests have been written (by **On entry:** read the root `CLAUDE.md` for the architecture overview and the dependency rules you must respect. +**Then read the target area's feature map** — the nested `features//CLAUDE.md` (and `domain//CLAUDE.md`, `data//CLAUDE.md` when relevant). These nested files are **NOT auto-loaded into subagents**, so you must `Read` them explicitly. Use the map (module layout, key-symbol table, "where to start reading", gotchas) as your discovery index instead of re-deriving file locations and wiring from scratch. If no feature map exists for the area, proceed with normal discovery. + **On exit:** finish with a HANDOFF block (template `.claude/docs/agent-toolkit/templates/HANDOFF.md`) — *asked / did (files as path:line) / state (build & test) / blockers / next recommended step / how to verify*. Your verdict maps to "state" + "next recommended step". ## Part 1: Code Verification diff --git a/.claude/docs/agent-toolkit/README.md b/.claude/docs/agent-toolkit/README.md index 8ab94ccc19..665ee72e68 100644 --- a/.claude/docs/agent-toolkit/README.md +++ b/.claude/docs/agent-toolkit/README.md @@ -22,6 +22,32 @@ This contract is the whole answer to "a user can resume at any time with minimal each HANDOFF block makes its step legible cold, so the orchestrator (and a human) can synthesize where things stand and what to do next. +## Standing conventions every agent follows (audit these) +These exist because agents were burning time and context. `agent-auditor` should flag any agent +that violates them. + +1. **Findings go in the HANDOFF, not on disk.** No agent writes scratch analysis/design `.md` + files to `.claude/docs/` (or anywhere) unless the user explicitly asks for a persisted + document by name. Long on-disk dumps bloat the repo and get truncated by context compaction — + the opposite of resumable. Keep HANDOFFs tight: links and `path:line`, not prose. +2. **Never fight the build's automation.** detekt runs with `autoCorrect = true` + + `detekt-formatting` (see `plugins/configuration/.../DetektConfigurations.kt`), so the whole + Formatting rule set is auto-fixed by running the task. Agents must not hand-edit + autocorrectable violations. Generally: if a Gradle task fixes something, run it — don't + reimplement it by hand. +3. **Iterate on the fast task, verify on the slow one.** Use compile-only tasks + (`compile*UnitTestKotlin`, `compile*Kotlin`) to catch errors; run the full test/detekt task + once, filtered (`--tests`, single module), to confirm. Never re-run a slow task per fix. +4. **The repo's own rule files are the source of truth.** e.g. `.claude/rules/unit-testing.md` + for tests. Agents point to them rather than duplicating (and drifting from) their content. +5. **Specialists read the feature map before discovering.** Nested `features//CLAUDE.md` + files (the curated per-feature code maps: module layout, key-symbol table, gotchas) are + **NOT auto-loaded into subagents** — only the root hierarchy is. Every specialist's entry + contract must `Read` the target area's `features//CLAUDE.md` (and `domain/`/`data/` + counterparts) when it exists, and use it as the discovery index. This is what stops the same + production hubs (`SwapModel`, `DefaultSendComponent`, …) being re-mapped from scratch every + run. `code-analyzer` flags areas that lack a map so one can be created. + ## Contents ``` agent-toolkit/ diff --git a/.claude/docs/agent-toolkit/templates/CHANGE-SET.md b/.claude/docs/agent-toolkit/templates/CHANGE-SET.md new file mode 100644 index 0000000000..ffdd1ed52b --- /dev/null +++ b/.claude/docs/agent-toolkit/templates/CHANGE-SET.md @@ -0,0 +1,79 @@ +# CHANGE-SET block (the apply contract) + +> **Why this exists.** In this environment, an `Agent` subagent runs detached. It cannot +> surface an interactive permission prompt, so any `Write` / `Edit` — or a non-allowlisted +> Bash write (`cat >`, `touch`, `>>`) — it attempts is **auto-denied**. Allowlisted Bash +> (e.g. `./gradlew …`) still runs fine. Therefore **subagents never write files.** A +> specialist designs the change and returns it as a CHANGE-SET; the **main loop** (Claude +> Code itself) applies it with `Write`/`Edit`, which is where the user approves each write. +> +> A specialist returns a CHANGE-SET *in addition to* its HANDOFF when its job was to produce +> file changes. Read-only specialists (code-analyzer, verifier, reviewers) return only a +> HANDOFF. + +The block must be **deterministically applyable** — the main loop should be able to apply it +mechanically without re-deriving anything. Give exact paths and exact text. + +``` +## CHANGE-SET — — + +**Summary:** one line — what this set of changes accomplishes. + +### Apply order +1. (new) +2. (edit) +3. register module in settings.gradle.kts + …list every item below in the order the main loop should apply them… + +### New files +For each new file: full path, then the COMPLETE file content in a fenced block. + +#### `path/to/NewFile.kt` +```kotlin + +``` + +### Edits to existing files +For each edit: the path, then one or more (old → new) pairs. `old_string` must be an +EXACT, UNIQUE substring of the current file (enough surrounding lines to be unambiguous) so +the main loop can apply it with the `Edit` tool verbatim. No line-number-only references. + +#### `path/to/Existing.kt` +- old: + ```kotlin + + ``` + new: + ```kotlin + + ``` + +### Deletes / renames / bash +Explicit shell commands (the main loop runs them): `git mv …`, `rm …`, etc. + +### Post-apply verification (run by the main loop AFTER applying) +The exact commands to confirm the change-set is correct. You could NOT compile it yourself — +the files were not on disk during your run — so list precisely what must be checked: +```bash +./gradlew :features::impl:compileDebugKotlin +./gradlew :features::impl:detekt +./gradlew :features::impl:testDebugUnitTest --tests "" +``` + +### Risks / assumptions +Anything you could not verify read-only (a symbol you assumed exists, an API shape you +inferred). The main loop checks these first if verification fails. +``` + +## Rules for producing a good CHANGE-SET + +- **No elisions.** New files are complete; edits carry enough context to be unique. A `// …` + or `/* unchanged */` placeholder makes the set un-applyable — never use one. +- **Edits target the smallest unique anchor**, not whole-file rewrites, so the diff stays + reviewable and the `Edit` apply is unambiguous. +- **Front-load discovery read-only.** You cannot compile un-applied code, so verify every + symbol, import, package path, and API shape against the *current* tree with Read/Grep + before you commit it to the set. Wrong assumptions surface as build failures after apply, + which costs a full round-trip — minimise them. +- **State what you could not verify** in *Risks / assumptions*. Honesty here is what lets the + main loop fix a failed apply in one step instead of re-investigating from scratch. \ No newline at end of file