diff --git a/AGENTS.md b/AGENTS.md index 2afbd9f..04d9574 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,7 +15,7 @@ agents/ flutter-reviewer.md # Read-only Flutter code reviewer subagent docs/ # Gitignored, local only plan/ # Planning and design documents -evals/ # `claude plugin eval` suite — all 15 skills, 100 cases +evals/ # `claude plugin eval` suite — all 15 skills, 101 cases README.md # Case format, grader reference, how to add a case _fixture/ fixture.sh # The only copy of the neutral Flutter skeleton; every case symlinks here diff --git a/evals/README.md b/evals/README.md index 8643b46..14b83dc 100644 --- a/evals/README.md +++ b/evals/README.md @@ -4,7 +4,7 @@ Does Claude route to the skill, and does the output follow it? Cases run through [`claude plugin eval`](https://code.claude.com/docs/en/plugin-evals) against real models. ```bash -claude plugin eval . --scaffold # all 100 cases, both arms +claude plugin eval . --scaffold # all 101 cases, both arms claude plugin eval . --scaffold --tag bloc # one skill claude plugin eval . --scaffold --ablation none # with-plugin arm only, half the cost ``` @@ -105,7 +105,7 @@ made to a [mocked MCP tool](#mocking-the-mcp-servers). ### Routing graders -Ninety-nine of the hundred cases carry one: +A hundred of the 101 cases carry one: ```markdown --- @@ -305,7 +305,7 @@ plugin and 0.00 without, on three runs each. ```bash E="claude plugin eval . --scaffold" -$E # all 100, both arms +$E # all 101, both arms $E --tag bloc --tag testing # two skills $E --case bloc-writes-sealed-events-and-states # one case $E --ablation none # with-plugin arm only, half the cost diff --git a/evals/layered-architecture/NOTES.md b/evals/layered-architecture/NOTES.md index c3269f9..54335a4 100644 --- a/evals/layered-architecture/NOTES.md +++ b/evals/layered-architecture/NOTES.md @@ -63,6 +63,23 @@ sharing one definition does remove duplication. **Discriminates.** A bare model reaches for a service locator or a global singleton, and lists the repository packages as hosted pub dependencies. +### layered-architecture-injects-client-the-app-pubspec-declares + +**Discriminates.** The prompt asks for an app pubspec that lists only the repository. A bare +model honors that by giving `WeatherRepository` a `baseUrl` and building the client inside +it, through a redirecting `: this._(WeatherApiClient(baseUrl: baseUrl))` or an initializer +list, and says outright that it does so to keep `weather_api_client` out of the app. The +skill requires the client in the constructor, constructs it in `main_development.dart`, and +declares `weather_api_client` in the app pubspec because that entrypoint imports it. + +**Grader notes.** `repository-builds-no-client` and `app-pubspec-declares-client` carry the +case and weigh 2 each. `repository-builds-no-client` catches a `??` default, an +initializer-list construction, and a redirecting constructor, and stays clear of the +`final weatherApiClient = WeatherApiClient(...)` line in `main`. +`app-pubspec-declares-client` anchors on `path: packages/` so the repository pubspec's +`path: ../weather_api_client` cannot satisfy it, and allows a leading `+` because the +plugin arm often answers with a diff. + ### layered-architecture-stays-out-of-single-file-work **Discriminates.** No `packages/`, `_api_client`, `_repository` or `RepositoryProvider` diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/case.yaml b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/case.yaml new file mode 100644 index 0000000..431edf5 --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/case.yaml @@ -0,0 +1,4 @@ +schema_version: "1.1" +name: layered-architecture-injects-client-the-app-pubspec-declares +context: + scaffold_script: fixture.sh diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/fixture.sh b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/fixture.sh new file mode 120000 index 0000000..56543ee --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/fixture.sh @@ -0,0 +1 @@ +../../_fixture/fixture.sh \ No newline at end of file diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/app-pubspec-declares-client.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/app-pubspec-declares-client.md new file mode 100644 index 0000000..ff0bec6 --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/app-pubspec-declares-client.md @@ -0,0 +1,5 @@ +--- +type: regex +pattern: 'weather_api_client:\s*\+?\s*path:\s*(\./)?packages/weather_api_client' +weight: 2 +--- diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/constructs-client-in-main.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/constructs-client-in-main.md new file mode 100644 index 0000000..6b9e21a --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/constructs-client-in-main.md @@ -0,0 +1,7 @@ +--- +type: llm +--- + +PASS if lib/main_development.dart constructs a WeatherApiClient with the dev base URL and passes it into the WeatherRepository constructor. + +FAIL if main_development.dart constructs WeatherRepository without passing it a WeatherApiClient, or if the WeatherApiClient is constructed inside the repository package, for example in a factory constructor or a default parameter value. diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/repository-builds-no-client.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/repository-builds-no-client.md new file mode 100644 index 0000000..b6a0dea --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/repository-builds-no-client.md @@ -0,0 +1,6 @@ +--- +type: regex +pattern: '(\?\?|[:,]\s*_\w+\s*=|this\.\w*\()\s*WeatherApiClient\(' +match: not_contains +weight: 2 +--- diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/requires-client.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/requires-client.md new file mode 100644 index 0000000..7ed4bb8 --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/requires-client.md @@ -0,0 +1,4 @@ +--- +type: regex +pattern: 'required\s+WeatherApiClient\s+\w+' +--- diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/skill-fired.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/skill-fired.md new file mode 100644 index 0000000..5791dd4 --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/skill-fired.md @@ -0,0 +1,7 @@ +--- +type: tool_used +tool: Skill +input_match: '"skill"\s*:\s*"(?:[\w-]+:)?layered-architecture"' +weight: 3 +arm: both +--- diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/prompt.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/prompt.md new file mode 100644 index 0000000..c20bfbb --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/prompt.md @@ -0,0 +1,26 @@ +--- +max_turns: 12 +timeout_seconds: 600 +allowed_tools: [Read, Glob, Grep, Skill] +tags: [layered-architecture] +description: "Bootstrap injection under a request to keep the app pubspec to repositories only: the repository requires its client and never builds one, main_development.dart constructs it, and the app pubspec declares the data package." +--- + +My monorepo has a data layer package at `packages/weather_api_client`. It is not in your working directory, so treat this as its exact public API: + +```dart +class WeatherApiClient { + WeatherApiClient({required String baseUrl}); + + Future getWeather(String city); +} + +class WeatherResponse { + const WeatherResponse({required this.city, required this.temperature}); + + final String city; + final double temperature; +} +``` + +Create `packages/weather_repository` with a `WeatherRepository` whose `getWeather(String city)` maps the response to a `Weather` domain model, and wire it into the app so features can reach it. I want the app's pubspec to list only `weather_repository` so it stays clean. The dev API is at `https://api.dev.example.com`. Show the Dart code for the repository and `lib/main_development.dart`, and the app pubspec changes. diff --git a/skills/layered-architecture/SKILL.md b/skills/layered-architecture/SKILL.md index 3094e74..5fd9432 100644 --- a/skills/layered-architecture/SKILL.md +++ b/skills/layered-architecture/SKILL.md @@ -26,7 +26,7 @@ Layered monorepo architecture for Flutter apps — four layers organized as inde Apply these standards to ALL layered architecture work: -- **Four layers** — Data, Repository, Business Logic, Presentation — every feature spans exactly these four layers +- **Four layers** — Data, Repository, Business Logic, Presentation — a feature spans all four whenever its repository reads an external source - **Unidirectional dependencies** — Presentation → Business Logic → Repository → Data — never skip or invert a layer - **Data and Repository layers live in `packages/`** — each is an independent Dart package with its own `pubspec.yaml` - **Business Logic and Presentation live in `lib/`** — organized by feature within the app @@ -36,8 +36,9 @@ Apply these standards to ALL layered architecture work: - **One repository per domain** — `user_repository`, `weather_repository`, `auth_repository` - **Path dependencies for local packages** — never `git:` or pub version references for packages in the same repo - **Barrel exports at every package boundary** — `src/` is never imported directly by consumers -- **Repositories accept data layer dependencies via constructor injection** — never instantiate clients internally +- **Repositories take every external source through the constructor** — a data client or an SDK object such as `FirebaseAuth.instance`, built in bootstrap and passed in, never constructed or defaulted inside the repository - **App bootstrap wires all layers** — `main_.dart` creates clients and repositories, provides them via `RepositoryProvider` +- **Only the app's entrypoints and bootstrap import a data package** — blocs, widgets, and app tests reach data through a repository - **Dart 3.13 primary constructors** — on the Dart 3.13+ baseline (`very_good_core` 1.6 / `very_good_analysis` 11), declare model and widget fields as primary-constructor declaring parameters (`class const User(final String id, final String name) extends Equatable`) rather than `this.field`; keep the classic form only below 3.13 > **Cross-harness fallback.** This skill scaffolds and tests packages via the Very Good CLI MCP server. On a host without this plugin's Bash hooks and without that MCP server connected, run the equivalent `very_good create dart_package …`, `very_good packages get`, and `very_good test` commands directly. @@ -47,7 +48,7 @@ Apply these standards to ALL layered architecture work: | Layer | Responsibility | Location | Depends On | Example | | --- | --- | --- | --- | --- | | **Data** | External communication — API calls, local storage, platform plugins | `packages/_api_client/` | External packages only | `user_api_client`, `local_storage_client` | -| **Repository** | Data orchestration — combines data sources, transforms models, caches | `packages/_repository/` | Data layer packages | `user_repository`, `weather_repository` | +| **Repository** | Data orchestration — combines data sources, transforms models, caches | `packages/_repository/` | Zero or more data layer packages | `user_repository`, `weather_repository` | | **Business Logic** | State management — processes user actions, emits state changes | `lib//bloc/` or `lib//cubit/` | Repository layer | `LoginBloc`, `ProfileCubit` | | **Presentation** | UI — widgets, pages, views, layout | `lib//view/` | Business Logic layer | `LoginPage`, `ProfileView` | @@ -172,14 +173,14 @@ See [worked-example.md](references/worked-example.md) for the complete `user_api ## Repository Layer -The repository layer orchestrates data sources and exposes domain models. Each repository composes one or more data clients, transforms response models into domain models, and provides a clean API for the business logic layer. +The repository layer orchestrates data sources and exposes domain models. Each repository composes the data clients it needs, transforms response models into domain models, and provides a clean API for the business logic layer. **Rules:** - No inter-repository dependencies — repositories are isolated - No Flutter SDK — the `very_good_cli` MCP server `create dart_package` tool - Domain models live in the repository package — not in data packages - Transform data models into domain models — never leak API response shapes upstream -- Accept all data clients via constructor injection +- A repository with no external source, such as in-memory session state, takes no constructor arguments ### Pattern: Domain Model + Repository Transformation @@ -225,13 +226,16 @@ See [worked-example.md](references/worked-example.md) for the complete `user_rep ## Dependency Graph -Path dependencies in each `pubspec.yaml` are what enforce the architecture. A data package -declares external packages only. A repository package declares a path dependency on its data -package. The root app declares **repository packages only** — data packages arrive as -transitive dependencies. +Path dependencies in each `pubspec.yaml` point one direction. A data package declares +external packages only. A repository package declares a path dependency on its data +packages. The root app declares its repository packages and every data package its bootstrap +constructs. `main_.dart` imports each client to inject it, and +`depend_on_referenced_packages` in `package:very_good_analysis` requires every imported +package to be declared. -**The app never depends on a data package directly.** That is the boundary: business logic -and presentation cannot bypass the repository layer, because they cannot import past it. +**Imports hold the layer boundary.** Once the app declares a data package, the lint no +longer stops a bloc from importing it. The boundary is the import rule in Core Standards: +only the app's entrypoints and bootstrap import a data package. ```yaml # packages/user_api_client/pubspec.yaml — external packages only @@ -243,13 +247,16 @@ dependencies: user_api_client: path: ../user_api_client -# pubspec.yaml — repository packages only; data packages are transitive +# pubspec.yaml: repositories, plus the data packages bootstrap constructs dependencies: + user_api_client: + path: packages/user_api_client user_repository: path: packages/user_repository ``` -See [references/pubspec.md](references/pubspec.md) for the three files in full. +See [references/pubspec.md](references/pubspec.md) for the three files in full and for +checking the import boundary, including which files count as entrypoints and bootstrap. ## Data Flow @@ -281,12 +288,19 @@ Future _onLoadRequested( ## App Bootstrap -`main_.dart` constructs every data client and repository, then passes them to the -`App` widget, which exposes them through `MultiRepositoryProvider`. Flavors change only -configuration — base URLs, API keys — never the wiring shape. +`main_.dart` imports and constructs every data client and repository, then passes +the repositories to the `App` widget, which exposes them through `MultiRepositoryProvider`. +Flavors change only configuration — base URLs, API keys — never the wiring shape. ```dart // lib/main_development.dart +import 'package:auth_api_client/auth_api_client.dart'; +import 'package:auth_repository/auth_repository.dart'; +import 'package:flutter/material.dart'; +import 'package:my_app/app/app.dart'; +import 'package:user_api_client/user_api_client.dart'; +import 'package:user_repository/user_repository.dart'; + void main() { WidgetsFlutterBinding.ensureInitialized(); @@ -352,11 +366,12 @@ the `App` widget with `MultiRepositoryProvider`. ### Connecting a Repository to a Feature -1. Add path dependency on the repository package to root `pubspec.yaml` -2. Create the repository in `main_.dart` and pass it to `App` +1. Add path dependencies on the repository package and on each data package it takes to root `pubspec.yaml` +2. Create the data clients and the repository in `main_.dart` and pass the repository to `App` 3. Add `RepositoryProvider.value` in `App`'s `MultiRepositoryProvider` 4. Create the Bloc/Cubit with the repository injected — see the **bloc** skill 5. Build the Page/View with `BlocProvider` and `BlocBuilder` — see the **bloc** skill +6. Grep `lib/` and `test/` for `package:/` imports. The work is done when every hit is an entrypoint or bootstrap file, as the pubspec reference's Import Boundary section defines them ## Additional Resources diff --git a/skills/layered-architecture/references/pubspec.md b/skills/layered-architecture/references/pubspec.md index 0097f63..5c01b65 100644 --- a/skills/layered-architecture/references/pubspec.md +++ b/skills/layered-architecture/references/pubspec.md @@ -61,10 +61,16 @@ dependencies: flutter: sdk: flutter flutter_bloc: ^9.1.0 + auth_api_client: + path: packages/auth_api_client auth_repository: path: packages/auth_repository + user_api_client: + path: packages/user_api_client user_repository: path: packages/user_repository + weather_api_client: + path: packages/weather_api_client weather_repository: path: packages/weather_repository @@ -105,8 +111,8 @@ dev_dependencies: ## Layer Boundaries in `pubspec.yaml` -Each layer's `pubspec.yaml` is what actually enforces the architecture. Path dependencies -point one direction only. +Path dependencies point one direction only. The root app is the one package that declares +both layers, because its bootstrap constructs the data clients it injects into repositories. ### Data Package (`packages/user_api_client/pubspec.yaml`) @@ -134,9 +140,45 @@ dependencies: flutter: sdk: flutter flutter_bloc: ^9.1.0 - # Repository packages only — data packages are transitive + # Repositories, plus each data package main_.dart constructs + auth_api_client: + path: packages/auth_api_client auth_repository: path: packages/auth_repository + user_api_client: + path: packages/user_api_client user_repository: path: packages/user_repository ``` + +## The Import Boundary + +With the data packages declared, `depend_on_referenced_packages` no longer flags a bloc that +imports one. The boundary is the import rule: only the app's entrypoints and bootstrap import +a data package. + +The entrypoints and bootstrap are the files that construct the clients. In a Very Good CLI +app they are `lib/main_.dart` and `lib/bootstrap.dart`. A single-flavor app has +`lib/main.dart`. Some apps group them in a directory, such as `lib/main/main_development.dart` +and `lib/main/bootstrap/bootstrap.dart`. Treat whichever files the app uses as the allowed set. + +Check the boundary after wiring a repository. Grep `lib/` and `test/` for each data package's +imports: + +```text +^(import|export) 'package:user_api_client/ +``` + +The check passes when every hit is an entrypoint or bootstrap file. Fix any other hit in +the code: + +- A bloc or widget that names a data-layer model, enum, or exception gets a domain type + instead. The repository defines it and maps to it, the way `UserRepository` catches + `UserApiException` and throws its own `UserNotFoundException`. +- A bloc or widget that calls the client calls a repository method instead. +- An app test mocks the repository, never the client. + +When the data layer is a third-party SDK used without a wrapper package, such as +`package:firebase_auth` or `package:firebase_storage`, the same rule covers its imports. Its +package name does not mark it as data layer, so add each SDK the repositories take to the +grep by name.