From 0fea2c04bc66a9b1abc04a78394cae286e7500f0 Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Thu, 16 Jul 2026 15:35:05 -0400 Subject: [PATCH 1/2] feat: add specialized review, documentation, and CLI skills plus known adopters resource --- .../skills/api-review/SKILL.md | 29 +++ .../references/canonical_api_design.md | 116 ++++++++++++ .../skills/code-documentation/SKILL.md | 69 +++++++ .../code-documentation/references/dart.md | 80 ++++++++ .../code-documentation/references/python.md | 111 +++++++++++ .../references/typescript.md | 86 +++++++++ .../code-review/references/reviewing_tests.md | 87 +++++++++ .../skills/unix-cli-best-practices/SKILL.md | 176 ++++++++++++++++++ .../resources/known_adopters.md | 10 + 9 files changed, 764 insertions(+) create mode 100644 .agents/agents/reidbaker-agent/skills/api-review/SKILL.md create mode 100644 .agents/agents/reidbaker-agent/skills/api-review/references/canonical_api_design.md create mode 100644 .agents/agents/reidbaker-agent/skills/code-documentation/SKILL.md create mode 100644 .agents/agents/reidbaker-agent/skills/code-documentation/references/dart.md create mode 100644 .agents/agents/reidbaker-agent/skills/code-documentation/references/python.md create mode 100644 .agents/agents/reidbaker-agent/skills/code-documentation/references/typescript.md create mode 100644 .agents/agents/reidbaker-agent/skills/code-review/references/reviewing_tests.md create mode 100644 .agents/agents/reidbaker-agent/skills/unix-cli-best-practices/SKILL.md create mode 100644 tool/dart_skills_lint/.agents/skills/dart-skills-lint-integration/resources/known_adopters.md diff --git a/.agents/agents/reidbaker-agent/skills/api-review/SKILL.md b/.agents/agents/reidbaker-agent/skills/api-review/SKILL.md new file mode 100644 index 0000000..98dbcea --- /dev/null +++ b/.agents/agents/reidbaker-agent/skills/api-review/SKILL.md @@ -0,0 +1,29 @@ +--- +name: api-review +description: Reviews the specified code against the canonical API Design guidelines. Use this skill when the user asks for an API review or to check code against API design principles. +--- + +# API review skill + +This skill reviews code against the canonical API Design guidelines. + +## Instructions + +1. **Load Guidelines**: Read the API design guidelines from [references/canonical_api_design.md](references/canonical_api_design.md) to ensure they are fully available in the context. +2. **Identify Target**: Identify the code to review. + - If the user specified files (e.g., "review main.dart"), use those. + - If the user has an open file in their context, assume that is the target. + - If neither, ask the user to specify the target files. +3. **Analyze**: For each target file, perform a deep analysis against the "Foundations of Canonical API Design Principles" (loaded in step 1), specifically looking for: + - **Contract-First**: Is the interface clear and decoupled from implementation? + - **KISS/YAGNI**: Are there unnecessary parameters or over-generalized features? + - **Ergonomics**: Are names intent-revealing? Do they follow the Principle of Least Astonishment? + - **CQS**: Are commands and queries separated? + - **Safety**: Are types used strictly (Enums vs Strings)? Is validation visible? + - **Explicit Configuration**: Are dependencies explicitly injected rather than implicitly resolved via global state, registries, or environment variables? +4. **Report**: Generate a structured report: + - **Score**: Give a letter grade (A-F) based on alignment. + - **Critical Issues**: Violations that _must_ be fixed (e.g., severe strictness or safety issues). + - **Suggestions**: Ergonomic improvements (renaming, rearranging). + - **Code Examples**: Provide `before` vs `after` code blocks for the suggested improvements. + - Save the report as a markdown artifact in the conversation artifacts directory (e.g., `/brain//api_review_results.md`). diff --git a/.agents/agents/reidbaker-agent/skills/api-review/references/canonical_api_design.md b/.agents/agents/reidbaker-agent/skills/api-review/references/canonical_api_design.md new file mode 100644 index 0000000..9b795fd --- /dev/null +++ b/.agents/agents/reidbaker-agent/skills/api-review/references/canonical_api_design.md @@ -0,0 +1,116 @@ +# Foundations of canonical API design principles + +This document contains the canonical principles for designing and improving APIs within the GenUI ecosystem. These principles are distilled from established software engineering best practices. + +## Table of contents + +- [Core architectural philosophy](#core-architectural-philosophy) + - [1. Contract-first design (API first)](#1-contract-first-design-api-first) + - [2. The KISS mandate (minimal surface area)](#2-the-kiss-mandate-minimal-surface-area) + - [3. YAGNI (avoid speculative generality)](#3-yagni-avoid-speculative-generality) + - [4. Separation of concerns (information hiding)](#4-separation-of-concerns-information-hiding) + - [5. Explicit configuration (dependency injection over stateful globals)](#5-explicit-configuration-dependency-injection-over-stateful-globals) +- [Interface ergonomics & semantics](#interface-ergonomics--semantics) + - [1. The principle of least astonishment (predictability)](#1-the-principle-of-least-astonishment-predictability) + - [2. Intent-revealing names](#2-intent-revealing-names) + - [3. Command-query separation (CQS)](#3-command-query-separation-cqs) + - [4. Orthogonality (composable primitives)](#4-orthogonality-composable-primitives) +- [Operational reliability & robustness](#operational-reliability--robustness) + - [1. Idempotency (safe retries)](#1-idempotency-safe-retries) + - [2. The robustness principle (Postel’s Law)](#2-the-robustness-principle-postels-law) + - [3. Circuit breaking (fail fast)](#3-circuit-breaking-fail-fast) + - [4. Bulk & batch operations](#4-bulk--batch-operations) +- [Data handling & evolution](#data-handling--evolution) + - [1. Scalable pagination (cursor over offset)](#1-scalable-pagination-cursor-over-offset) + - [2. Evolutionary design (versioning)](#2-evolutionary-design-versioning) + - [3. Type safety & strict validation](#3-type-safety-strict-validation) + - [4. Structured error reporting](#4-structured-error-reporting) + - [5. Single source of truth (DRY data)](#5-single-source-of-truth-dry-data) + +## Core architectural philosophy + +Foundational decisions that shape the system's structure and maintainability. + +### 1. Contract-first design (API first) + +Treat the interface definition as the primary product artifact, distinct from its implementation. Define the contract (the "what") rigorously before writing the code (the "how"). This decoupling allows consumers to validate the design against requirements before engineering resources are committed, and it serves as the single source of truth for the system's capabilities. + +### 2. The KISS mandate (minimal surface area) + +Complexity is the primary vector for defects and integration friction. Expose the smallest number of concepts necessary to solve the problem, aiming for "just enough power." Avoid "Swiss Army knife" interfaces that perform multiple unrelated operations. As a useful heuristic, if an interface requires extensive setup or explanation to use, it likely violates this principle. + +### 3. YAGNI (avoid speculative generality) + +Do not add parameters, operations, or data fields for hypothetical future needs. Speculative features introduce "accidental complexity"—complexity arising from the solution rather than the problem. Aim to build only what is required for current, concrete use cases to maintain a lean and testable interface. + +### 4. Separation of concerns (information hiding) + +The API must expose capabilities, not internal state or storage models. Never leak implementation details, such as database schemas or specific algorithms, through the interface. This encapsulation ensures that the internal architecture can be refactored or completely replaced without breaking consumers. + +### 5. Explicit configuration (dependency injection over stateful globals) + +Prefer explicit parameter passing and dependency injection over stateful global state, singletons, registries, or environment variables for API configuration. Implicit configurations obscure dependencies from caller visibility, prevent static type safety, and introduce global state mutations that make concurrent execution and parallel testing difficult. Programmatic instantiations are statically verifiable, isolate test cases, and make dependency compilation boundaries transparent to bundling tools. + +## Interface ergonomics & semantics + +Principles for ensuring the API is predictable, descriptive, and easy to use. + +### 1. The principle of least astonishment (predictability) + +An interface should behave exactly as a user expects based on their prior knowledge of the system. If a standard pattern exists for naming conventions or error structures, deviate from it only with significant justification. Consistency reduces the cognitive load required to learn and use the API. + +### 2. Intent-revealing names + +Names should describe business intent rather than technical mechanics. For example, a method named `publishArticle()` is superior to `updateRowStatus()`. Use specific, self-descriptive terminology, like `temperatureCelsius` instead of `temp`, to prevent ambiguity and reduce reliance on external documentation. + +### 3. Command-query separation (CQS) + +Clearly distinguish between operations that retrieve data (queries) and operations that modify state (commands). Queries should ideally be safe and side-effect-free, allowing consumers to call them repeatedly without risk. Avoid mixing data retrieval with state mutation in a single operation, as it leads to unpredictable system behavior. + +### 4. Orthogonality (composable primitives) + +Design small, independent primitives that can be composed to create complex behaviors, rather than creating rigid, complex operations for every specific scenario. Changing one independent parameter should not have unexpected side effects on unrelated parts of the request. + +## Operational reliability & robustness + +Designing for failure, distributed systems, and performance in a production environment. + +### 1. Idempotency (safe retries) + +In a distributed system, network failures are inevitable. Operations that change state must be designed to be idempotent, meaning they can be safely retried multiple times without producing cumulative side effects, such as double-charging. This is often achieved by requiring clients to provide a unique idempotency key or transaction identifier. + +### 2. The robustness principle (Postel’s Law) + +Follow the robustness principle: "Be conservative in what you send, be liberal in what you accept." Strictly adhere to the interface contract when generating output, but implement tolerance when processing input, such as ignoring unknown fields rather than crashing. This approach improves interoperability and allows the system to evolve without immediately breaking older clients. + +### 3. Circuit breaking (fail fast) + +If a dependency or subsystem is failing, stop calling it immediately to prevent cascading failures. The interface should fail fast and return a specific error rather than hanging indefinitely. This preserves system resources and provides immediate feedback to the consumer. + +### 4. Bulk & batch operations + +To reduce network overhead and latency, provide mechanisms to process multiple items in a single request, such as `createItems([item1, item2])`. Ensure the behavior of partial failures is well-defined: either the entire batch fails atomically, or the response clearly indicates the success or failure status of each individual item. + +## Data handling & evolution + +Guidelines for managing state, validation, and long-term interface maintenance. + +### 1. Scalable pagination (cursor over offset) + +Never return unbounded lists of data. For large datasets, prefer cursor-based (keyset) pagination over offset-based pagination. Cursors, which point to a specific record, provide constant-time performance (O(1)) and data stability. In contrast, offsets degrade in performance (O(n)) and suffer from skipped or duplicate records when data is modified during iteration. + +### 2. Evolutionary design (versioning) + +Plan for change from day one. Interfaces will evolve, and breaking changes must be managed without disrupting existing consumers. Use explicit versioning strategies, such as semantic versioning, and establish a clear deprecation lifecycle (warn, then sunset, and finally remove) to give consumers time to migrate. + +### 3. Type safety & strict validation + +Enforce constraints at the boundary. Use strong types, such as enums instead of raw strings, or distinct types for monetary values, to make invalid states unrepresentable. Validate all input rigorously before processing to prevent data corruption and security vulnerabilities. + +### 4. Structured error reporting + +Errors must be machine-readable and actionable. Return specific error codes, rather than generic failure indicators, accompanied by descriptive messages. Where possible, include structured metadata that allows the consumer to programmatically recover or correct their input. + +### 5. Single source of truth (DRY data) + +Avoid defining the same business logic or data structure in multiple places. Ensure that a concept, such as a user or an order, has a canonical representation. Divergent definitions lead to integration errors where one part of the system enforces rules that another ignores. diff --git a/.agents/agents/reidbaker-agent/skills/code-documentation/SKILL.md b/.agents/agents/reidbaker-agent/skills/code-documentation/SKILL.md new file mode 100644 index 0000000..7612948 --- /dev/null +++ b/.agents/agents/reidbaker-agent/skills/code-documentation/SKILL.md @@ -0,0 +1,69 @@ +--- +name: code-documentation +description: Guide for writing effective code documentation, including docstrings, JSDoc, dartdoc, and implementation comments. Use this skill when writing new code, adding features, or improving existing documentation in Dart, Python, or TypeScript to ensure clarity and maintainability. +--- + +# Code Documentation Skill + +This skill provides comprehensive guidelines for documenting code, prioritizing user-centric writing, clarity, and consistency. + +## 1. General Philosophy + +- **User-Centric**: Write for the person using your API. If you had to look up how to use something, document it so others don't have to. +- **Explain "Why"**: Explain _why_ code exists and _how_ to use it effectively, since the code signature already tells _what_ it does. +- **Be Concise**: Omit fluff. Avoid merely restating the code name, as it is not helpful. +- **Consistency**: Use standard terminology and consistent formatting. +- **Public APIs**: Document all public APIs (classes, members, top-level functions) without exception. +- **Code Samples**: Strongly consider adding code samples to explain usage. + +## 2. General Structure + +Follow this general structure for documentation comments across languages: + +1. **Summary Sentence**: Start with a single-sentence summary on the first line, ending with a period. +2. **Blank Line**: Follow the summary with a blank line. +3. **Details**: Add paragraphs, code samples, or lists as needed to explain parameters, return values, exceptions, and behavior. +4. **Annotations**: Place doc comments **before** any metadata annotations. + +## 3. Writing Guidelines + +### Brevity & Style + +- **Avoid Fluff**: Omit "This class...", "This method...", "Is used to...", "Note that...". + - _Bad_: "This method is used to calculate the total." + - _Good_: "Calculates the total." +- **Third-Person Verbs**: Start function/method docs with a third-person singular verb. + - _Examples_: "Returns...", "Calculates...", "Updates...", "Creates...". +- **Noun Phrases**: Start variable/property docs with a noun phrase. + - _Examples_: "The current color.", "A list of active users.". +- **Booleans**: Always start with "Whether" (or similar clear indicator). + - _Good_: "Whether this widget is enabled." + - _Bad_: "If this widget is enabled...", "True if...", "Flag to indicate...". +- **Avoid Jargon**: Use plain English unless the term is a widely accepted standard (e.g., "HTTP", "URL"). + +### Formatting + +- **Sparingly**: Use Markdown features (bold, lists) sparingly. +- **No HTML**: Avoid HTML unless strictly necessary and supported by the documentation tool. +- **Parameters/Returns/Exceptions**: Use prose to describe parameters, return values, and thrown exceptions. Do not rely solely on tags like `@param` unless mandated by the language standard (e.g., Javadoc). + +## 4. Implementation Comments + +Ensure implementation comments (`//`) are accurate, relevant, factual, and provide information that is not readily understandable from the code. Remove or reword comments that do not meet these criteria. If an implementation comment provides information useful to an API consumer that is not already in the documentation comments, move it to the documentation comments. + +## 5. Review Checklist + +Use this checklist to verify your documentation: + +1. [ ] **Summary**: Ensure every public member starts with a one-sentence summary ending in a period. +2. [ ] **Brevity**: Remove "This class..." or "This function..." fluff. +3. [ ] **Completeness**: Document strict constraints (e.g., "must not be null") and exceptions. +4. [ ] **Examples**: Consider adding a code sample for complex widgets or methods. + +## 6. Language Specific Instructions + +Refer to the language guides for detailed instructions on structure, linking, and framework-specific patterns: + +- **Dart / Flutter**: [references/dart.md](references/dart.md) +- **TypeScript / JavaScript**: [references/typescript.md](references/typescript.md) +- **Python**: [references/python.md](references/python.md) diff --git a/.agents/agents/reidbaker-agent/skills/code-documentation/references/dart.md b/.agents/agents/reidbaker-agent/skills/code-documentation/references/dart.md new file mode 100644 index 0000000..b640fb1 --- /dev/null +++ b/.agents/agents/reidbaker-agent/skills/code-documentation/references/dart.md @@ -0,0 +1,80 @@ +# Dart & Flutter Documentation + +## Documentation Structure (`///`) + +Use `///` (doc comments) for all public members to allow tools like `dartdoc` to process them. + +### Comments Format + +1. **Summary Sentence**: Start with a single-sentence summary on the first line, ending with a period. +2. **Blank Line**: Follow the summary with a blank line. +3. **Details**: Add paragraphs, code samples, or lists as needed to explain parameters, return values, exceptions, and behavior. +4. **Annotations**: Place doc comments **before** any metadata annotations (e.g., `@override`, `@Deprecated`). + +```dart +/// A button that initiates a purchase flow. +/// +/// This widget handles the loading state automatically and disables +/// itself while the transaction is processing. +@Deprecated('Use PurchaseButtonV2 instead') +class PurchaseButton extends StatelessWidget { ... } +``` + +### Property Documentation + +- **Getters override Setters**: Document the getter and omit documentation on the setter, as documentation tools combine them automatically. Do not document both. + ```dart + /// The current optimization level (0.0 to 1.0). + double get optimizationLevel => _level; + set optimizationLevel(double value) { ... } + ``` + +### Library Documentation + +- **Library Comments**: Add a doc comment at the top of the file (before imports) for libraries (files) to provide a high-level overview. + +## Writing Guidelines + +### Terminology + +- Use **canonical terms**: + - Refer to "widgets", "state", "build context", "render object". + - Avoid generic terms like "component" or "element" when you mean "Widget". + +### Code References + +- Use square brackets `[MyClass]`, `[variableName]`, `[methodName]` to link to in-scope identifiers. +- Use backticks `` `true` ``, `` `null` ``, `` `this` `` for keywords and literals. +- **Code Samples**: Include code blocks to demonstrate usage. + ````dart + /// Example: + /// + /// ```dart + /// final path = FlightPath(coordinates: [a, b]); + /// ``` + ```` + +## Flutter Specifics + +### Widgets + +- **Purpose**: Explain what the widget does and when to use it. +- **Parameters**: Document key parameters, especially if they are required or have complex constraints. +- **State**: Explain any interesting state behavior, such as keeping position on scroll. + +```dart +/// Displays a flight path on a map. +/// +/// Use this widget within a [MapLayout]. The path is drawn using +/// the provided [coordinates]. +class FlightPath extends StatelessWidget { + /// Creates a flight path. + /// + /// The [coordinates] must contain at least two points. + const FlightPath({required this.coordinates, super.key}); +``` + +### State Management + +- **Lifecycle**: Explain how to dispose of controllers or other classes that manage a complex lifecycle. +- **Private Classes**: Document significantly complex private classes (such as intricate `State` logic) to aid maintainability, even though public APIs are the priority. diff --git a/.agents/agents/reidbaker-agent/skills/code-documentation/references/python.md b/.agents/agents/reidbaker-agent/skills/code-documentation/references/python.md new file mode 100644 index 0000000..9f8f7db --- /dev/null +++ b/.agents/agents/reidbaker-agent/skills/code-documentation/references/python.md @@ -0,0 +1,111 @@ +# Python Documentation + +## Documentation Styles + +### Docstrings vs Comments + +- **Docstrings**: Use `""" ... """` for documentation that describes usage, arguments, and return values (public API) to make them accessible via `__doc__` and tools like `pydoc`. +- **Block/Inline Comments**: Use `#` for implementation details relevant only to developers reading the source. + +## Google Style Docstrings + +Follow the [Google Python Style Guide](https://google.github.io/styleguide/pyguide.html) for docstrings. + +### General Format + +- **Triple Double-Quotes**: Always use `"""`. +- **Summary Line**: Write a concise summary (max 80 chars) on the first line, ending with a period. +- **Blank Line**: Follow the summary with a blank line. +- **Sections**: Use specific headers for arguments, return values, and exceptions. + +```python +def fetch_bigtable_rows(table_handle, other_silly_variable=None): + """Fetches rows from a Bigtable. + + Retrieves rows pertaining to the given keys from the Table instance + represented by table_handle. String keys will be UTF-8 encoded. + + Args: + table_handle: An open smalltable.Table instance. + other_silly_variable: Another optional variable, that has a much + longer name than the other args, and which does nothing. + + Returns: + A dict mapping keys to the corresponding table row data + fetched. Each row is represented as a tuple of strings. For + example: + + {'Serak': ('Rigel VII', 'Preparer'), + 'Zim': ('Irk', 'Invader'), + 'Lrrr': ('Omicron Persei 8', 'Emperor')} + + If a row is not found, it is not included in the dict. + + Raises: + IOError: An error occurred accessing the bigtable.Table object. + """ + pass +``` + +### Sections Detailed + +- **Args**: List each parameter by name, followed by its description. +- **Returns**: Describe the return value (or use `Yields` for generators). You may omit this section if the function returns `None`, though explicit documentation is preferred. +- **Raises**: List all exceptions relevant to the interface. + +### Type Annotations + +- **Inline Types**: Omit type information from the docstring arguments if you use PEP 484 type hints (strongly recommended) to avoid duplication and drift. + + ```python + def greet(name: str) -> str: + """Returns a greeting. + + Args: + name: The person to greet. + + Returns: + The greeting string. + """ + return f"Hello, {name}" + ``` + +## Module Docstrings + +Provide a top-level docstring in every file describing its contents and usage. + +```python +"""A one-line summary of the module or program, terminating in a period. + +Leave one blank line. The rest of this docstring should contain an +overall description of the module or program. Optionally, it may also +contain a brief description of exported classes and functions and/or usage +examples. + + Typical usage example: + + foo = ClassFoo() + bar = foo.FunctionBar() +""" +``` + +## Class Docstrings + +Describe the class and its usage. Document public attributes here under an `Attributes` section. + +```python +class SampleClass: + """Summary of class here. + + Longer class information.... + + Attributes: + likes_spam: A boolean indicating if we like SPAM. + eggs: An integer count of the eggs we have. + """ + + def __init__(self, likes_spam: bool = False): + """Inits SampleClass with default values.""" + self.likes_spam = likes_spam + self.eggs = 0 +``` diff --git a/.agents/agents/reidbaker-agent/skills/code-documentation/references/typescript.md b/.agents/agents/reidbaker-agent/skills/code-documentation/references/typescript.md new file mode 100644 index 0000000..4f75948 --- /dev/null +++ b/.agents/agents/reidbaker-agent/skills/code-documentation/references/typescript.md @@ -0,0 +1,86 @@ +# TypeScript Documentation + +## Documentation Styles + +### JSDoc vs Implementation Comments + +- **Documentation**: Use `/** ... */` to document public APIs for users (JSDoc/TSDoc). +- **Implementation**: Use `//` for implementation details relevant only to developers reading the source code. +- **Block Comments**: Avoid `/* ... */` for multi-line comments. Use multiple `//` lines instead. + +```typescript +/** + * Computes the weight based on three factors. + * + * @param itemsSent The number of items sent. + * @param itemsReceived The number of items received. + */ +function computeWeight(itemsSent: number, itemsReceived: number): number { + // This is an implementation comment explaining the formula. + // It uses multiple lines of // comments. + return itemsSent * 2 + itemsReceived; +} +``` + +## JSDoc/TSDoc Format + +### General Form + +- **Single-line**: Use single-line format for short descriptions: `/** This is a short description. */` +- **Multi-line**: Use multi-line format for longer descriptions: + ```typescript + /** + * Multiple lines of JSDoc text are written here, + * wrapped normally. + * + * @param arg A number to do something to. + */ + ``` + +### Markdown + +Write JSDoc comments using **Markdown**. + +- Use standard Markdown for lists, code blocks, and formatting. +- Avoid plain text formatting that relies on whitespace, as tools will ignore it. + +```typescript +/** + * Computes weight based on three factors: + * + * - items sent + * - items received + * - last timestamp + */ +``` + +### Tags + +- **@param**: Place `@param` on its own line, followed by the parameter name and its description. +- **@return**: Place `@return` on its own line. +- **@deprecated**: Add this tag for deprecated symbols. + +## What to Document + +### Top-Level Exports + +Document all top-level exports of modules (classes, interfaces, functions, constants). + +- **Exception**: You may exempt symbols exported only for tooling (e.g., Angular `@NgModule` classes) if their purpose is obvious. + +### Classes + +Provide enough information/context for the reader to know **how and when** to use the class. + +- Omit textual descriptions on the constructor if the class documentation already covers it. + +### Methods and Functions + +- **Purpose**: Explain what the function does. +- **Parameters/Return**: Document parameters and return values if they are not immediately obvious from types and names. +- **Redundancy**: Avoid merely restating the name (e.g., `@param name The name` is useless). + +## Tooling Compatibility + +- Ensure JSDoc structures are valid so that TypeScript-aware editors (VS Code) and documentation generators (TypeDoc) can process them. +- Ensure comments are well-formed to enable strict type checking and tool support. diff --git a/.agents/agents/reidbaker-agent/skills/code-review/references/reviewing_tests.md b/.agents/agents/reidbaker-agent/skills/code-review/references/reviewing_tests.md new file mode 100644 index 0000000..4b2f28b --- /dev/null +++ b/.agents/agents/reidbaker-agent/skills/code-review/references/reviewing_tests.md @@ -0,0 +1,87 @@ +# Reviewing Tests + +This reference document outlines guidelines for reviewing and evaluating unit, integration, and other tests during code reviews to ensure coverage, reliability, and correctness. + +To make test reviews effective: + +- Keep pull requests small and focused. Avoid reviewing tests in large, multi-feature PRs. +- For complex or high-risk features, consider interactive review walkthroughs with developers and testers to align unit, integration, and acceptance tests. + +## Architectural boundaries and test doubles + +Unit, integration, and end-to-end tests should have distinct purposes: + +- **Avoid redundant overlap**: Do not assert the same business outcomes across all levels. +- **Share utilities**: Reuse test data generation factories and helper objects across test suites. +- **Isolate unit tests**: Use test doubles to replace external dependencies (such as databases and APIs) to keep tests fast and deterministic. + +Minimize the use of mocks. AI tools and developers often over-rely on mocks, which leads to fragile tests that pass even when integration points are broken. Use the most appropriate testing double: + +| Double type | Purpose | Verification style | Review standard | +| :----------------- | :-------------------------------------------------------------- | :---------------------- | :------------------------------------------------------------------------ | +| **Fake** | Lightweight, working implementation (e.g., in-memory database). | State verification | Preferred. Minimizes external dependencies without mock setup. | +| **Stub** | Returns hardcoded responses to specific calls. | State verification | Good for controlling specific test inputs. | +| **Mock** | Verifies specific method interactions and call counts. | Behavioral verification | Limit to 3-4 per test. Only mock I/O boundaries. | +| **Spy** | Wraps a real object to record calls while executing real logic. | Hybrid verification | Avoid unless verifying interactions with immutable third-party libraries. | +| **Dummy** | Empty object passed to satisfy type signatures. | None | Preferred for clean setup when the dependency is unused. | + +## Test smells + +Flag test smells (poor testing practices) during code reviews. Unaddressed smells cause test suites to become flaky, slow, and hard to maintain. + +| Smell classification | Specific anti-pattern | Diagnostic indicator | Impact | +| :--------------------- | :-------------------- | :------------------------------------------------------------------------------ | :------------------------------------------------------ | +| **Structural** | Mystery Guest | Relies on external files, databases, or configuration not declared in the test. | Environment-dependent failures, cannot run in parallel. | +| **Structural** | Eager Test | Verifies multiple distinct functional concepts in a single test block. | Hard to diagnose failures. | +| **Behavioral** | Assertion Roulette | Multiple assertions in a test block without custom failure messages. | First failure halts execution, hiding other failures. | +| **Behavioral** | For Testers Only | Modifying production code API solely to make it testable. | Compromised production API design. | +| **Maintenance** | Sleepy Test | Using hardcoded delays or sleep statements. | Slow execution, race conditions. | +| **Maintenance** | Sensitive Equality | Assertions that fail on minor, irrelevant formatting changes. | Fragile tests that break on minor edits. | +| **Maintenance** | Dead Test | Tests with missing or trivial assertions. | False confidence in test coverage. | +| **Organizational** | Test Maverick | Fails to follow project testing conventions. | Readability and onboarding issues. | +| **Organizational** | General Fixture | Setup code loads unrelated data models and tables. | Slow execution, database flakiness. | + +## SDK contracts, web UI, and dynamic waits + +Apply specific standards when reviewing tests for libraries, SDKs, or web interfaces: + +- **SDK tests**: Verify public attributes, return types, method chaining, and default values. When introducing breaking changes, write tests that assert deprecation warnings are emitted and verify legacy behavior. +- **UI selectors**: Use accessibility-aware selectors (like roles or labels) rather than absolute page layouts or CSS class structures. +- **Waits**: Use asynchronous, dynamic conditional waiting (polling until state is met) instead of hardcoded sleep intervals. + +## Generative AI and deterministic verification + +AI-generated tests can create a coverage illusion by asserting current code behavior—including existing bugs—rather than the actual business requirements. + +Use a structured workflow for AI-assisted tests: + +1. **Behavioral specification**: Draft empty test blocks defining expectations before generating code. +2. **AI generation**: Use AI to write boilerplate setup, mocks, and execution logic. +3. **Human review**: Evaluate the assertions against business requirements. Ask if the tests would pass if a bug were introduced. +4. **Mutation testing**: Run mutation tools (such as StrykerJS, mutmut, or pitest) to verify that tests catch changes to production code. + +To avoid manual mocking and flaky behavior, consider deterministic verification tools (like BitDive or Skyramp) that capture and replay database and network calls. Keep a centralized, repository-managed registry of regression test cases to train and update automated verification agents. + +## Review matrix + +Use the following checklist and questions to evaluate test files: + +| Target | Self-reflection question | Intended check | +| :------------------------------------ | :---------------------------------------------------------------- | :------------------------------------------------------------------- | +| **Functional correctness** | Does the test confirm acceptance criteria and cover edge cases? | Identifies missing Happy Path or Failure Path scenarios. | +| **Refactoring integrity** | Would the test still pass if the internal implementation changed? | Identifies tests tightly coupled to specific implementation details. | +| **Failure predictability** | If a bug were deliberately introduced, would this test fail? | Identifies weak assertions or dead tests. | +| **Boundary security** | Are inputs validated and permission boundaries asserted? | Identifies missing security validation. | +| **Concurrency safety** | Does the test execute safely in a parallel thread environment? | Identifies race conditions and deadlocks. | +| **State isolation** | Does the test clean up mutations to avoid leaking state? | Identifies flaky tests that fail when run concurrently. | + +### Open-ended questions + +Ask yourself these questions during code review: + +- "What is the reasoning behind this specific test architecture or mock setup?" +- "Are there performance, database query count, or latency impacts associated with this setup?" +- "How does this testing strategy align with our team's conventions and standards?" +- "What changes would improve the readability and maintainability of this test structure?" +- "What testing strategy do you recommend for high-risk safety requirements?" +- "As a skeptical engineer, where will the tests pass when they shouldn't?" diff --git a/.agents/agents/reidbaker-agent/skills/unix-cli-best-practices/SKILL.md b/.agents/agents/reidbaker-agent/skills/unix-cli-best-practices/SKILL.md new file mode 100644 index 0000000..293acb8 --- /dev/null +++ b/.agents/agents/reidbaker-agent/skills/unix-cli-best-practices/SKILL.md @@ -0,0 +1,176 @@ +--- +name: unix-cli-best-practices +description: Safe, portable, and efficient command-line patterns for macOS/BSD Unix tools (grep, find, sed, awk, xargs, mdfind, pbcopy, open) and modern alternatives (ripgrep, fd). Covers common shell scripting and one-liner use cases including fast searching, text processing, codebase navigation, and parallel execution. +--- + +# Unix CLI best practices + +This guide provides efficient, safe, and portable techniques for manipulating text and finding files from the command line on macOS. By default, macOS uses BSD-derived UNIX utilities (`grep`, `find`, `sed`, `awk`) which have subtle differences from their GNU/Linux counterparts. + +When possible, use high-performance replacements (`ripgrep` and `fd`) which respect `.gitignore` rules by default and offer more ergonomic interfaces. + +## Modern high-performance alternatives + +Use these tools instead of standard BSD utilities for local codebase operations. + +### ripgrep (`rg`) + +Use `ripgrep` (`rg`) as the preferred tool for searching text. It is faster than standard `grep` and respects `.gitignore` rules by default. + +Common use cases: +* Run `rg 'search_term'` to recursively search the current directory. +* Run `rg -S 'term'` to search case-insensitively unless the query contains uppercase letters. Use `rg -i 'term'` for strict case-insensitivity. +* Run `rg -g '*.ts' -g '!*.spec.ts' 'term'` to search `.ts` files while excluding `.spec.ts` files. +* Run `rg -v 'ignore_me'` to print lines that do not match the pattern. +* Run `rg -l 'term'` to list only the names of matching files. Combine with `rg -0` for null-terminated output suitable for `xargs`. +* Run `rg -F 'foo()'` to treat the search pattern as a fixed string rather than a regular expression. +* Run `rg -w 'const'` to match whole words only. +* Run `rg -C 2 'term'` to show two lines of surrounding context. Use `rg -B 2` for preceding context or `rg -A 2` for succeeding context. + +Refer to `rg --help` for the complete options list. + +### fd + +Use `fd` as the preferred tool for traversing the filesystem and finding files. It is faster than standard `find` and respects `.gitignore` rules by default. + +Common use cases: +* Run `fd 'pattern'` to find files matching the regex pattern anywhere in their path. +* Run `fd -e py -e txt` to filter results by file extensions. +* Run `fd -t d 'docs'` to find directories. Use `fd -t f` for files and `fd -t l` for symbolic links. +* Run `fd -H` to include hidden files, or `fd -I` to include ignored files (such as `node_modules`). Combine as `fd -HI` to search all files. +* Run `fd -p 'src/assets'` to match the pattern against the full path instead of just the filename. +* Run `fd -e log -x rm` to execute a command on each matching file individually. Use `-X` (e.g., `fd -e log -X rm`) to run the command once with all matching files as arguments. +* Run `fd -a 'pattern'` to return absolute paths instead of relative paths. + +Refer to `fd --help` for the complete options list. + +## Built-in tools for macOS and BSD + +Use these built-in utilities only when `rg` and `fd` are unavailable. Apply precise filters to prevent performance degradation. + +### Efficient searching with grep + +Do not use `grep | grep -v 'unwanted_dir'` to exclude directories. This approach reads all files before filtering them. Use native exclusion arguments instead to exclude directories at the filesystem level: + +```bash +# Slow: reads binary and ignored files +grep -R "pattern" . | grep -v "node_modules" + +# Fast: skips the directory completely at the filesystem level +grep -R --exclude-dir=node_modules --exclude-dir=.git "pattern" . +``` + +Common `grep` flags: +* `-r` or `-R` to search recursively. `-R` follows symbolic links, while `-r` does not. +* `-I` to ignore binary files for faster execution and clean output. +* `--exclude="*.min.js"` to ignore files matching a specific glob. +* `--include="*.dart"` to search only files matching a specific glob. +* `-l` to print only the names of files containing matches. +* `-Z` to print a null character after each filename, which is useful when piping to `xargs -0`. + +Example of safely deleting files containing a specific string: +```bash +grep -rlZ -I --exclude-dir=node_modules "DEPRECATED_API" . | xargs -0 rm +``` + +### High-performance traversal with find + +Prevent `find` from traversing irrelevant directory trees by using the `-prune` option. + +```bash +# Slow: traverses node_modules entirely, then filters output +find . -name "*.ts" | grep -v "node_modules" + +# Fast: skips node_modules entirely +find . -name "node_modules" -prune -o -name "*.ts" -print +``` + +The expression `-name "node_modules" -prune` stops traversal when encountering a `node_modules` directory. The `-o` (OR) operator specifies that for any other directory or file ending in `.ts`, the path is printed. + +### macOS portability with sed + +macOS uses BSD `sed`, which differs from GNU `sed` in its handling of in-place editing. + +Use the `-i` option with an explicit backup extension. To edit in-place without creating a backup, provide an empty string: + +```bash +# Edit in-place without creating a backup +sed -i '' 's/oldName/newName/g' filename.txt + +# Edit in-place and create a backup named filename.txt.bak +sed -i '.bak' 's/oldName/newName/g' filename.txt +``` + +Use the `-E` option to enable extended regular expressions, avoiding the need to escape parentheses and plus signs: +```bash +sed -E 's/(foo|bar)+/baz/g' file.txt +``` + +### Codebase analysis with awk + +Use `awk` for line-by-line data extraction and text processing. + +Common use cases: +* Print specific columns from space-separated input: + ```bash + ls -l | awk '{print $1, $3}' + ``` +* Find and print duplicate lines in a file without sorting them first: + ```bash + awk '!seen[$0]++' filename.txt + ``` +* Filter lines based on column values (for example, printing lines where the third column is greater than 100): + ```bash + awk '$3 > 100' data.txt + ``` +* Find lines matching a pattern and print their line number (`NR`) along with the content: + ```bash + awk '/Error/ {print NR, $0}' server.log + ``` + +### Safe pipelines and parallelism with xargs + +Use `xargs` to build and execute commands from standard input. Always use the `-0` option (null-terminated) when processing file paths to handle filenames containing spaces or special characters safely. + +```bash +# Unsafe: will fail or perform unintended actions if filenames contain spaces +find . -name "*.log" | xargs rm + +# Safe: handles spaces and special characters correctly +find . -name "*.log" -print0 | xargs -0 rm +``` + +Use the `-P` option to run tasks in parallel: +```bash +# Run up to 4 curl processes in parallel +cat urls.txt | xargs -n 1 -P 4 curl -O +``` + +### macOS-specific CLI tools + +Use macOS-specific utilities to interact with operating system features: + +* Use `mdfind` to query the macOS Spotlight index for fast, global file and content searches without scanning the disk: + ```bash + # Search by filename + mdfind -name "project_spec" + + # Search by text content within files + mdfind "kMDItemTextContent == 'TODO: Refactor'" + ``` +* Use `pbcopy` and `pbpaste` to interact with the system clipboard: + ```bash + # Copy file content to the clipboard + cat ssh_key.pub | pbcopy + + # Paste clipboard content to a file + pbpaste > new_file.txt + ``` +* Use `open` to open files, directories, or URLs with their default applications: + ```bash + # Open the current directory in Finder + open . + + # Open a local HTML file using a specific application + open -a "Google Chrome" index.html + ``` diff --git a/tool/dart_skills_lint/.agents/skills/dart-skills-lint-integration/resources/known_adopters.md b/tool/dart_skills_lint/.agents/skills/dart-skills-lint-integration/resources/known_adopters.md new file mode 100644 index 0000000..bedc490 --- /dev/null +++ b/tool/dart_skills_lint/.agents/skills/dart-skills-lint-integration/resources/known_adopters.md @@ -0,0 +1,10 @@ +# Known Adopters of dart_skills_lint + +The following repositories use `dart_skills_lint` to validate their agent skills. + +- [flutter/flutter](https://github.com/flutter/flutter) +- [flutter/devtools](https://github.com/flutter/devtools) +- [flutter/packages](https://github.com/flutter/packages) +- [dart-lang/skills](https://github.com/dart-lang/skills) +- [dart-lang/site-www](https://github.com/dart-lang/site-www) +- [kevmoo/dart-best-practices](https://github.com/kevmoo/dart-best-practices) From eb69c2e00a1c7965d9a829eddce8edce607c0ae1 Mon Sep 17 00:00:00 2001 From: Reid Baker <1063596+reidbaker@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:37:38 -0400 Subject: [PATCH 2/2] Delete tool/dart_skills_lint/.agents/skills/dart-skills-lint-integration/resources/known_adopters.md --- .../resources/known_adopters.md | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 tool/dart_skills_lint/.agents/skills/dart-skills-lint-integration/resources/known_adopters.md diff --git a/tool/dart_skills_lint/.agents/skills/dart-skills-lint-integration/resources/known_adopters.md b/tool/dart_skills_lint/.agents/skills/dart-skills-lint-integration/resources/known_adopters.md deleted file mode 100644 index bedc490..0000000 --- a/tool/dart_skills_lint/.agents/skills/dart-skills-lint-integration/resources/known_adopters.md +++ /dev/null @@ -1,10 +0,0 @@ -# Known Adopters of dart_skills_lint - -The following repositories use `dart_skills_lint` to validate their agent skills. - -- [flutter/flutter](https://github.com/flutter/flutter) -- [flutter/devtools](https://github.com/flutter/devtools) -- [flutter/packages](https://github.com/flutter/packages) -- [dart-lang/skills](https://github.com/dart-lang/skills) -- [dart-lang/site-www](https://github.com/dart-lang/site-www) -- [kevmoo/dart-best-practices](https://github.com/kevmoo/dart-best-practices)