diff --git a/matrix/domain-coverage.md b/matrix/domain-coverage.md new file mode 100644 index 0000000..6388c36 --- /dev/null +++ b/matrix/domain-coverage.md @@ -0,0 +1,46 @@ +# Domain Coverage + +> Where each core principle applies across the domains. The complement to `principles-matrix.md`: the matrix maps domain → core; this maps core → domains. + +## Core Principle Coverage + +| Core Principle | Domains that derive from it | Count | +|----------------|---------------------------|-------| +| C1 Correctness | All 11 (security, data, api, testing, performance, observability, errors, uiux, documentation, concurrency, devops) | Universal | +| C2 Clarity | uiux, api, data, testing, observability, errors, documentation, devops | 8 | +| C3 Simplicity | security, data, testing, performance, documentation, concurrency, devops | 7 | +| C4 Locality | testing, concurrency | 2 | +| C5 Reversibility | api, data, uiux, concurrency, devops | 5 | +| C6 Composability | api, security, observability, errors, documentation, concurrency | 6 | +| C7 Observability | api, data, testing, performance, observability, errors, devops | 7 | +| C8 Economy | security, testing, performance, observability, concurrency | 5 | + +## Interpretation + +- **C1 Correctness** is universal — every domain derives from it. This confirms C1 as the floor. +- **C4 Locality** is the narrowest (2 domains: testing independence, concurrency boundaries). Locality is inherently domain-specific; not a gap. +- **C2 Clarity** and **C7 Observability** are the next-most-derived, confirming they are cross-cutting concerns. +- **C6 Composability** appears in 6 domains — it is most relevant where parts combine (api, errors, observability, documentation, concurrency) and least relevant where the unit is monolithic. + +## Per-Domain Coverage + +| Domain | C-rules derived from | Notes | +|--------|---------------------|-------| +| UI/UX | C1, C2, C3, C5, C7 | User-facing; clarity + reversibility | +| API | C1, C2, C3, C5, C6, C7 | Contracts; composability + stability | +| Security | C1, C3, C6, C7, C8 | 8/10 non-tradeable (promoted to C1) | +| Data | C1, C2, C3, C5, C7 | Truth + migration safety | +| Testing | C1, C2, C3, C4, C7, C8 | Independence (C4) is unique | +| Performance | C1, C3, C6, C7, C8 | Economy + measurement | +| Observability | C1, C2, C6, C7, C8 | Self-referential (P7 derives from C7) | +| Errors | C1, C2, C5, C6, C7 | Errors as data + reversibility | +| Documentation | C1, C2, C3, C5, C6 | Docs-as-code + living docs | +| Concurrency | C1, C3, C4, C5, C6, C7, C8 | Broadest derivation; locality (C4) unique | +| DevOps | C1, C2, C3, C5, C7 | Reproducibility + rollback | + +## Gaps and Notes + +- No domain derives from only one C-rule. The minimum is 4 (UI/UX: C1, C2, C3, C5, C7 — actually 5). Every domain is multi-rooted. +- **Concurrency** has the broadest derivation (7 C-rules) — it touches the most core concerns. +- **UI/UX** and **API** are the most user-facing; they emphasize C2 (Clarity) heavily. +- **Security** is the only domain with explicit non-tradeable declarations; this promotes 8 of its rules to C1-equivalent per `core/conflict-resolution.md` §6. \ No newline at end of file diff --git a/review/agent-checklist.md b/review/agent-checklist.md new file mode 100644 index 0000000..6055121 --- /dev/null +++ b/review/agent-checklist.md @@ -0,0 +1,135 @@ +# Agent Pre-Completion Checklist + +> Every AI agent runs this checklist before completing a task. If any item fails, fix it before finishing. This is the gate between "the code is written" and "the task is done." + +## How to Use This + +1. Read the relevant `domains//first-principles.md` before starting the task. +2. Implement the task. +3. Run this checklist. Every item must pass (or be explicitly justified). +4. If an item fails, fix it. Do not "skip" without a written reason. + +## Core Principles Checklist (C1–C8) + +### C1 Correctness +- [ ] Does the code do what the task asked, completely? +- [ ] Does it handle the specified edge cases? (nulls, empties, max, min) +- [ ] Does it handle the failure cases? (errors, timeouts, invalid input) +- [ ] Is there a test that would fail if the code were wrong? + +### C2 Clarity +- [ ] Can a stranger read this and understand it without asking you? +- [ ] Are names intent-revealing? (No `data`, `temp`, `x`, `doStuff`) +- [ ] Do comments explain *why*, not *what*? +- [ ] Is the structure scannable? (Short functions, clear sections) + +### C3 Simplicity +- [ ] Is this the simplest solution that is complete? +- [ ] Is there dead code? (Unreachable branches, unused variables) +- [ ] Is there premature abstraction? (An interface with one implementation) +- [ ] Could 50 lines do what 200 lines do? + +### C4 Locality +- [ ] Does related logic live together? +- [ ] Are side effects near their causes? +- [ ] Does a change to this feature require touching distant files? + +### C5 Reversibility +- [ ] Is this change undoable? (migration has a `down`, deploy has a rollback) +- [ ] Did I avoid irreversible actions without explicit confirmation? +- [ ] Is state recoverable? (Can the user get back to where they were?) + +### C6 Composability +- [ ] Does this component/function do one thing? +- [ ] Is the boundary (props/args/return) explicit and typed? +- [ ] Can this be reused in a new context without modification? + +### C7 Observability +- [ ] Are there logs for significant events? +- [ ] Do errors carry enough context to debug? (request ID, user, action) +- [ ] Are there metrics for the operation? (count, latency) +- [ ] Are there no secrets in logs? + +### C8 Economy +- [ ] Is memory bounded? (No unbounded growth, no loading everything) +- [ ] Is time bounded? (No N+1, no blocking without timeout) +- [ ] Are resources released? (file handles, connections, locks) + +## Domain-Specific Triggers + +If the task touches a domain, run that domain's checklist: + +### If UI/UX (see `domains/uiux/`) +- [ ] Every interactive element is keyboard-reachable +- [ ] Every image has alt text (or marked decorative) +- [ ] Every form control has a label +- [ ] Focus is visible +- [ ] No color-only information +- [ ] Components use design tokens, not raw values + +### If API (see `domains/api/`) +- [ ] Endpoints are nouns, plural, lowercase-hyphenated +- [ ] Status codes are correct (200/201/204/4xx/5xx per semantics) +- [ ] Errors are structured (code, message, request_id) +- [ ] Input is validated against a schema +- [ ] Auth is required by default + +### If Security (see `domains/security/`) +- [ ] No secrets in code, logs, URLs, or error messages +- [ ] Input is validated at the boundary +- [ ] Output is encoded for its context +- [ ] Crypto uses vetted libraries (no MD5/SHA1 for security) +- [ ] Authorization is checked, not assumed + +### If Data (see `domains/data/`) +- [ ] Schema reflects the domain (not the application) +- [ ] Constraints are in the schema (NOT NULL, UNIQUE, FK) +- [ ] Migration has an `up` and a `down` +- [ ] Types are domain-accurate (UUID, TIMESTAMPTZ, DECIMAL for money) +- [ ] No `SELECT *`; no N+1 + +### If Testing (see `domains/testing/`) +- [ ] Tests are independent (order doesn't matter) +- [ ] Tests are deterministic (no `Date.now()`, no `random()`) +- [ ] Edge cases are covered (empty, single, max, invalid) +- [ ] A failing test names the problem specifically + +### If Performance (see `domains/performance/`) +- [ ] No unbounded operations (loops, allocations, queries) +- [ ] No N+1 queries +- [ ] Every external call has a timeout +- [ ] Caches have invalidation strategies + +### If Observability (see `domains/observability/`) +- [ ] Logs are structured (JSON, fields) +- [ ] Every request has a correlation ID +- [ ] No high-cardinality labels in metrics +- [ ] Alerts have runbooks + +### If Errors (see `domains/errors/`) +- [ ] Errors are not swallowed silently +- [ ] Errors are specific (not generic "something went wrong") +- [ ] Errors preserve context (where, when, why, what) +- [ ] Recovery is attempted when possible; fail fast when not + +### If Concurrency (see `domains/concurrency/`) +- [ ] Shared state is minimized; immutability preferred +- [ ] Locks are minimal in scope +- [ ] Queues are bounded +- [ ] Every blocking call has a timeout +- [ ] Cancellation is supported + +### If DevOps (see `domains/devops/`) +- [ ] The pipeline is the process (no manual steps) +- [ ] Rollback path is known +- [ ] Config is in code, not on the server +- [ ] Environments are parity (dev = prod modulo data) + +## Final Gate + +- [ ] Have I read the relevant domain's first-principles? +- [ ] Have I run the domain-specific checklist? +- [ ] Have I run the core checklist? +- [ ] Are all failures either fixed or explicitly justified in the task notes? + +If any unchecked item is not justified, the task is not complete. Do not mark done. \ No newline at end of file diff --git a/review/anti-patterns.md b/review/anti-patterns.md new file mode 100644 index 0000000..e3b092f --- /dev/null +++ b/review/anti-patterns.md @@ -0,0 +1,132 @@ +# Anti-Patterns + +> A catalog of violations. Each entry names the principle it breaches. Use this to recognize and reject patterns on sight. + +## How to Use + +When you see a pattern listed here, it is a defect. Cite the principle it violates and require a fix. These are not "to be reviewed later"; they are rejected on sight. + +## Core Anti-Patterns (C1–C8) + +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| Code that "mostly works" | C1 Correctness | Mostly correct is incorrect | +| `function doStuff()` | C2 Clarity | Name reveals nothing | +| A 500-line function | C3 Simplicity | Complexity is a liability | +| Config in a distant repo, read silently | C4 Locality | Coupling you cannot see | +| A migration with no `down` | C5 Reversibility | Irreversible bet | +| A component reading global state implicitly | C6 Composability | Hidden dependency | +| A service with no logs | C7 Observability | Cannot debug what you cannot see | +| Loading all records into memory | C8 Economy | Unbounded = OOM | + +## Domain Anti-Patterns + +### UI/UX +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| Image without `alt` | P2 Accessibility | Disqualifying | +| "Delete" with no confirmation | P5 Forgiveness | Irreversible surprise | +| `color: #3b82f6` in a component | P8 Consistency (via tokens) | Bypasses design system | +| "Submit" on a delete button | P3 Clarity | Wrong verb | +| Layout shift on image load | P7 Hierarchy / CLS | Visual instability | + +### API +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| `/getUsers` (verb in URL) | P2 Clarity | Resources are nouns | +| 200 with an error body | P9 Error Transparency | Status code lies | +| 500 with a stack trace | P8 Security, P9 | Information leak | +| No `Idempotency-Key` on a POST | P6 Idempotency | Retry is unsafe | +| 10MB response by default | P7 Performance | Unbounded payload | + +### Security +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| `eval()` of any string | P4, P5 | Code injection | +| Hardcoded API key in source | P9 Secret Hygiene | Committed secret | +| `catch (e) {}` (swallow) | P7, P8 | Silent failure, fail-open | +| `md5` for password hashing | P6 Crypto | Broken primitive | +| Open CORS `*` in production | P1, P10 | Zero trust violated | +| `chmod 777` | P2 Least Privilege | Maximum privilege | +| Logging the request body | P9 Secret Hygiene | Token leak | + +### Data +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| `VARCHAR` for a UUID | P7 Type Fidelity | Wrong type | +| No FOREIGN KEY | P3, P9 | Unenforced relationship | +| `FLOAT` for money | P7, P1 | Floating point error | +| `is_deleted` without filtering | P8 Lifecycle | Soft-delete leak | +| `SELECT *` | P10 Performance | Unbounded columns | + +### Testing +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| Test that cannot fail | P10 No Test Theater | Not a test | +| `Date.now()` in a fixture | P3 Determinism | Flaky | +| Shared fixture mutated across tests | P2 Independence | Order-dependent | +| 500 e2e tests, 50 unit | P4 Fast Feedback | Inverted pyramid | +| `name: "test"` fixture | P7 Realism | Hides bugs | + +### Performance +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| N+1 query in a loop | P3 Complexity | O(N) queries | +| No timeout on HTTP call | P4, P8 (concurrency) | Hang forever | +| Cache with no invalidation | P5 Caching | Stale forever | +| Unbounded in-memory sort | P4 Resource Bounds | OOM | +| Optimization without measurement | P1 Measure First | Guesswork | + +### Observability +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| `console.log("here")` | P1 Structured | Not queryable | +| `user_id` as a metric label | P4 Cardinality | Unbounded bill | +| Average latency only | P8 SLO | Hides the tail | +| No `trace_id` propagation | P2 Correlation | Cannot trace | +| Logs without `request_id` | P3 Context | No correlation | + +### Errors +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| `catch (e) { return null }` | P2 Fail Loudly | Silent failure | +| `throw new Error("error")` | P3 Fail Specifically | Generic | +| `return null` for "not found" | P1 Errors are Data | Conflates absence with error | +| Retry without backoff | P5, P8 | Retry storm | +| `throw` in a recovery path | P6 | Fail fast in wrong place | + +### Documentation +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| Doc with no examples | P3 Examples | Incomplete | +| Stale doc (wrong, not updated) | P4 Currency | Worse than no doc | +| Unlisted doc (not in MANIFEST) | (framework rule) | Not part of framework | +| No audience statement | P2 Audience | Who is this for? | + +### Concurrency +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| Shared mutable state, no lock | P1 Immutability | Race condition | +| Unbounded queue | P9 Bounded Queues | OOM | +| `channel.send()` with no timeout | P8 Timeout | Hang | +| Mutex held across I/O | P3 Lock Scope | Lock too long | +| Spawned work with no cancellation | P7 Cancellation | Orphaned work | + +### DevOps +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| Manual deploy script | P2 Automation | Not repeatable | +| No rollback path | P4 Rollback | Irreversible deploy | +| Big-bang deploy | P5 Progressive | All-or-nothing | +| Rebuild per environment | P7 Immutability | Different artifacts | +| Snowflake server | P1, P6 | Not reproducible | + +## Cross-Cutting Anti-Patterns + +| Anti-Pattern | Breaches | Why | +|--------------|----------|-----| +| "I'll add tests later" | C1 (no proof of correctness) | Later never comes | +| "It's just a prototype" | C5 (irreversible by default) | Prototypes go to prod | +| Copy-paste code | C6 (no composition) | Bug duplicated | +| God object | C3, C6 | One thing, many things | +| Leaky abstraction | C6, C2 | Hidden coupling | \ No newline at end of file diff --git a/review/peer-review-checklist.md b/review/peer-review-checklist.md new file mode 100644 index 0000000..8a23bbe --- /dev/null +++ b/review/peer-review-checklist.md @@ -0,0 +1,82 @@ +# Peer Review Checklist + +> For human reviewers. Run this when reviewing a PR or a change. Complements `agent-checklist.md` (which the author ran before finishing). + +## Purpose + +A peer review is a second set of eyes on correctness, clarity, and completeness. It is not a gatekeeping ritual; it is a quality multiplier. The author ran the agent checklist; the reviewer checks what the author could not see. + +## The Review + +### Understanding (read first, judge never) +- [ ] Read the PR description. What is the change trying to do? +- [ ] Read the linked issue/spec. Does the change address it? +- [ ] Read the changed files in order. Do not jump to judgment. + +### Correctness (C1) +- [ ] Does the change do what it claims? +- [ ] Are there edge cases the author missed? (Comment, don't fix in review) +- [ ] Are there failure cases unhandled? +- [ ] Are the tests testing the right thing? (A test that cannot fail is theater) +- [ ] Would this code fail in production? (Different from "does it pass CI") + +### Clarity (C2) +- [ ] Can you understand the change without asking the author? +- [ ] Are names intent-revealing? +- [ ] Is there a comment that explains *why* where the *why* is non-obvious? +- [ ] Is the diff minimal? (Unrelated changes are review noise) + +### Simplicity (C3) +- [ ] Is there dead code introduced? +- [ ] Is there premature abstraction? +- [ ] Could the change be simpler? (Suggest, don't block unless egregious) +- [ ] Is there a simpler approach the author may not have considered? + +### Locality (C4) +- [ ] Are related changes grouped? +- [ ] Are unrelated changes separated (different PRs)? +- [ ] Does a change require touching distant files unnecessarily? + +### Reversibility (C5) +- [ ] Is the change undoable? +- [ ] Are there migrations? Do they have rollbacks? +- [ ] Are there breaking changes? If so, is there a deprecation path? + +### Composability (C6) +- [ ] Does the change respect existing boundaries? +- [ ] Does it introduce coupling that should be an interface? +- [ ] Is the new code reusable, or one-off? + +### Observability (C7) +- [ ] Are there logs/metrics for the new behavior? +- [ ] Are errors structured and traceable? +- [ ] Are there no secrets in logs? + +### Economy (C8) +- [ ] Are there unbounded operations? +- [ ] Is memory/time bounded? +- [ ] Is the cost proportional to the need? + +## Domain-Specific (if applicable) + +Run the relevant domain section from `agent-checklist.md` (UI/UX, API, Security, Data, Testing, Performance, Observability, Errors, Concurrency, DevOps). The author ran it; the reviewer verifies. + +## Review Etiquette + +- **Comment, don't command.** "This could be X" not "Change this to X." +- **Distinguish blocking from suggestions.** "Blocking: correctness bug. Suggestion: naming." +- **Praise good code.** Reviews are not just for finding problems. +- **Don't review style the linter should catch.** Fix the linter, not the PR. +- **Ask questions.** "Why this approach?" often reveals more than "This is wrong." + +## Approving + +- Approve when: the change is correct, clear, and complete. Minor suggestions can be left for the author. +- Request changes when: there is a correctness bug, a missing test, or a clarity problem that blocks understanding. +- Reject when: the change should not exist (wrong direction, duplicate, scope-violating). + +## What This Checklist is Not + +- Not a style guide. Style is automated (linter, formatter). +- Not a gatekeeping tool. The goal is quality, not perfection. +- Not a substitute for the author's own checklist. The author runs `agent-checklist.md` first; this is the second pass. \ No newline at end of file