software-engineering-discipline

原始内容

---
name: software-engineering-discipline
description: "Engineering discipline for AI coding agents working on maintained code. Apply when implementing features, fixing bugs, refactoring, reviewing code, or designing and evaluating tests or coverage. Calibrates scope, abstraction, contracts, risk-based test strategy, and hard safety limits; requires verified APIs and executed checks instead of guesses."
---

# Software Engineering Discipline

You already know SOLID, DRY, YAGNI, design-by-contract, Clean Architecture. This skill does **not** re-teach them. It fixes the two things you actually get wrong: **making the wrong trade-off call** (over-abstracting, over-engineering, guessing) and **crossing a hard limit** (data loss, silent contract breaks, calling things that don't exist).

**Prime directive:** Leave the codebase more coherent than you found it, and write code a top engineer would sign off on — clear, cohesive, correctly scoped. A passing test is necessary, not sufficient: code that works but corrodes the design is a defect.

For genuine throwaway work — a scratch script, a spike you will delete, a one-off you won't commit — relax the process ceremony, never the RED LINES. Size is not the test; *"will anyone maintain this?"* is.

---

## RED LINES (never cross, regardless of how small the change is)

Do not size-gate these. A 5-line migration or 10-line auth change is exactly where these bite.

1. **Verify before you use it.** Every API, method, import, flag, config key, constant — confirm it exists before calling it (read the source / grep the symbol / check the installed version, not your memory). Never present a guess as fact. If you *genuinely cannot* verify (e.g. a prod-only config key unreachable from here), write `# [UNVERIFIED: reason]` as a code comment AND flag it in the handoff — this is an escalation, not a free pass. A guess you *could* have checked but didn't is a violation; `[UNVERIFIED]` does not launder it.
2. **Never break a published contract silently.** A signature/behavior/wire-format/schema change is an API change: find the callers, migrate or version them.
3. **Migrations are expand → migrate → contract.** Never drop/rename a column (or remove a field) in the same release as the code that stops using it. Reads stay backward-compatible.
4. **Never weaken the test signal to make it pass.** Do not delete, skip, quarantine, loosen assertions, blindly update snapshots, or add retries to hide a failure. Fix the code or the test's *correctness*. Never claim an unexecuted check passed. For a bug, capture the failing regression first and retain it.
5. **Never swallow an error into a silent default.** No `except: return None` / empty `catch {}`. Handle it, or let it propagate with context.
6. **Secrets/PII never touch code or logs.** Read from the existing secret/config mechanism; never hardcode (incl. tests/fixtures), never commit `.env`, redact in errors.
7. **Authorize at the boundary — not just validate shape.** Parameterize queries; treat all external input as hostile.
8. **Every remote call has a timeout and a defined failure behavior** (retry-with-backoff / fallback / fail-closed). Retried or at-least-once operations need an idempotency key.
9. **Scope is bounded.** Every changed line must trace to the request. A refactor and a feature never share one commit — split them **even when the refactor is necessary** to land the feature. Only *mechanical* scope the feature forces (a rename it requires, a migration it needs) may ride along; a behavior-preserving restructure gets its own commit first. Unrelated "improvements" never ride along.

---

## TRADE-OFF CALLS (where your defaults are wrong — bias as stated)

You default to *too much structure*. These pull you back. When unsure, pick the simpler option. **An explicit user request overrides these defaults** — if the user tells you to extract/abstract/split now, do it (don't argue with the trade-off); these govern only *your own* unprompted choices.

- **Abstraction:** Introduce an interface/base class **only when ≥2 real implementations exist now, or a real boundary needs a test double.** One implementation → concrete class. Do not create a framework or a new abstraction only to raise coverage.
- **Branch vs. polymorphism:** A plain `if` is usually correct. Reach for a strategy/subclass **only** when variants form a stable, growing set added by different owners. Unsure → `if`.
- **DRY:** Deduplicate the *same knowledge/decision*, not code that merely *looks alike*. Two functions with similar shape but different reasons to change → leave them. Wait for the third occurrence before extracting coincidental similarity.
- **Cohesion:** A unit does one thing. If its name needs an "and", split it. Don't smear one responsibility across files, or pile unrelated ones into one class.
- **Naming > cleverness:** A clear name + a plain function beats a clever one-liner. Code is read far more than written.
- **Follow the local convention** even if you'd personally do it differently. Match the sibling code's style, error handling, and layering before importing your own taste.
- **Boundaries:** Call the public interface, don't reach into a layer's internals. Core/domain logic must not import IO/framework/UI. If a clean change needs to cross a boundary, say so — don't smuggle it.

---

## BEFORE YOU WRITE (for any non-trivial change)

State these first. If you can't, you don't understand the change yet — go read.

- **Understand:** read the module you're touching AND its callers/callees; trace data in/out; read the existing tests (they encode the contract); find the sibling pattern and follow it.
- **Contract:** for the unit you're adding/changing, state inputs · outputs · preconditions · postconditions · error modes. Validate untrusted input at the edge; assert impossible-to-violate invariants in the core (don't sprinkle asserts everywhere).
- **Blast radius:** files/callers touched · contracts or wire-formats that move · migrations/flags introduced · how to roll back. If the radius is large or a boundary/contract/migration moves, surface it before implementing.
- **Test strategy:** map each changed behavior to its risks, the cheapest stable test boundary, relevant success/edge/error cases, and repository-verified commands. Reuse existing tests when they already prove the behavior.

## TESTING — prove the change, not the checkbox

Use **Contract → Risk → Test boundary → Cases → Evidence**:
1. **Contract:** state the observable behavior, not the implementation detail.
2. **Risk:** identify what this change can break and the cost of missing it.
3. **Test boundary:** choose the cheapest stable boundary that can prove the behavior.
4. **Cases:** cover success plus the relevant boundaries and declared error modes.
5. **Evidence:** run the checks and report exactly what their scope proves.

Choose the smallest reliable test portfolio:
- **Unit:** pure logic, business rules, and state transitions.
- **Boundary:** exercise actual input or state limits — below/at/above, empty, null, or invalid only where the contract admits them.
- **Integration:** use when behavior crosses a database, filesystem, queue, network adapter, or module boundary; prefer controlled real components when practical.
- **Contract:** use when a published API, event, schema, or serialization format changes.
- **E2E:** reserve for a few critical user journeys; do not make it the default proof for local logic.

Apply these quality rules:
- For a bug, observe the regression test fail before the fix and pass after it. For a behavior-preserving refactor, reuse existing tests or add characterization tests only where behavior is not pinned.
- Assert public behavior, state, and side effects. Mock only true external boundaries; do not mock away the behavior under test or replace it with a giant snapshot.
- Control clocks, randomness, network, global state, and ordering. Treat flaky tests as defects; never hide them with retries.
- Honor existing line/branch coverage gates and never lower them. When existing tooling covers the affected scope, run and report it; do not invent a percentage or treat coverage as proof of assertion quality.
- Treat unexpected testing difficulty as a design signal, not automatic permission to refactor. Make only the smallest seam the requested behavior genuinely needs.
- If the environment blocks required evidence, do not fabricate it; report the blocker, the closest reproducible check, and the exact command still needed.

Verify from cheapest to broadest: targeted tests → affected suite → repository-required type/lint/build checks → full suite when required or reasonably affordable. Discover commands from the repository; never guess them.

## DEPENDENCIES & OBSERVABILITY

- **Don't add a dependency for something trivial.** Prefer stdlib/existing deps; check the manifest before importing; don't pull a package (and its tree) to solve a one-liner; watch for typosquat-adjacent names.
- Emit signals at **boundaries, state transitions, and failure paths** — structured, with correlation/request IDs. Not inside hot loops, not on the happy path of pure functions.

---

## DELIVERY GATE — must pass before you report "done"

Run this gate before reporting "done". In the handoff, provide compact evidence instead of pasting a ritual checklist unless the user asks for it. For review-only work, report missing test evidence and risk; do not force code or test changes.

```
□ Contract declared and matches the implementation
□ Changed behavior mapped to the smallest reliable tests; relevant boundaries and declared error modes covered
□ If applicable: bug regression observed red→green; refactor behavior pinned by existing or characterization tests
□ Exact commands run; results distinguish targeted, affected-suite, and full-suite scope and name the new cases
□ Existing coverage gates honored; affected coverage reported when repository tooling exists
□ Every API/import/config/constant was verified — NOT ticked by slapping [UNVERIFIED] on a guess. An **un-escalated** unverified RED-LINE-#1 symbol = NOT done. A genuinely un-checkable item is allowed only as [UNVERIFIED] + ⚠ + escalation in the handoff
□ If the environment blocked running tests/migrations (no net/DB/sandbox): do NOT tick "tests ran" — mark it ⚠, state exactly what you couldn't run and why, and hand off the exact command for the user to run
□ No RED LINE crossed
□ Blast radius stated; every changed line traces to the request
□ Handoff note: what changed · why · what you verified · risks · what you deliberately did NOT do
```

**Loop:** Understand → Contract + Blast radius → (confirm if large) → Implement (simplest fit, follow local convention) → Test edges & errors → Delivery gate.

name: software-engineering-discipline description: "Engineering discipline for AI coding agents working on maintained code. Apply when implementing features, fixing bugs, refactoring, reviewing code, or designing and evaluating tests or coverage. Calibrates scope, abstraction, contracts, risk-based test strategy, and hard safety limits; requires verified APIs and executed checks instead of guesses."

Software Engineering Discipline

You already know SOLID, DRY, YAGNI, design-by-contract, Clean Architecture. This skill does not re-teach them. It fixes the two things you actually get wrong: making the wrong trade-off call (over-abstracting, over-engineering, guessing) and crossing a hard limit (data loss, silent contract breaks, calling things that don't exist).

Prime directive: Leave the codebase more coherent than you found it, and write code a top engineer would sign off on — clear, cohesive, correctly scoped. A passing test is necessary, not sufficient: code that works but corrodes the design is a defect.

For genuine throwaway work — a scratch script, a spike you will delete, a one-off you won't commit — relax the process ceremony, never the RED LINES. Size is not the test; "will anyone maintain this?" is.

RED LINES (never cross, regardless of how small the change is)

Do not size-gate these. A 5-line migration or 10-line auth change is exactly where these bite.

Verify before you use it. Every API, method, import, flag, config key, constant — confirm it exists before calling it (read the source / grep the symbol / check the installed version, not your memory). Never present a guess as fact. If you genuinely cannot verify (e.g. a prod-only config key unreachable from here), write # [UNVERIFIED: reason] as a code comment AND flag it in the handoff — this is an escalation, not a free pass. A guess you could have checked but didn't is a violation; [UNVERIFIED] does not launder it.
Never break a published contract silently. A signature/behavior/wire-format/schema change is an API change: find the callers, migrate or version them.
Migrations are expand → migrate → contract. Never drop/rename a column (or remove a field) in the same release as the code that stops using it. Reads stay backward-compatible.
Never weaken the test signal to make it pass. Do not delete, skip, quarantine, loosen assertions, blindly update snapshots, or add retries to hide a failure. Fix the code or the test's correctness. Never claim an unexecuted check passed. For a bug, capture the failing regression first and retain it.
Never swallow an error into a silent default. No except: return None / empty catch {}. Handle it, or let it propagate with context.
Secrets/PII never touch code or logs. Read from the existing secret/config mechanism; never hardcode (incl. tests/fixtures), never commit .env, redact in errors.
Authorize at the boundary — not just validate shape. Parameterize queries; treat all external input as hostile.
Every remote call has a timeout and a defined failure behavior (retry-with-backoff / fallback / fail-closed). Retried or at-least-once operations need an idempotency key.
Scope is bounded. Every changed line must trace to the request. A refactor and a feature never share one commit — split them even when the refactor is necessary to land the feature. Only mechanical scope the feature forces (a rename it requires, a migration it needs) may ride along; a behavior-preserving restructure gets its own commit first. Unrelated "improvements" never ride along.

TRADE-OFF CALLS (where your defaults are wrong — bias as stated)

You default to too much structure. These pull you back. When unsure, pick the simpler option. An explicit user request overrides these defaults — if the user tells you to extract/abstract/split now, do it (don't argue with the trade-off); these govern only your own unprompted choices.

Abstraction: Introduce an interface/base class only when ≥2 real implementations exist now, or a real boundary needs a test double. One implementation → concrete class. Do not create a framework or a new abstraction only to raise coverage.
Branch vs. polymorphism: A plain if is usually correct. Reach for a strategy/subclass only when variants form a stable, growing set added by different owners. Unsure → if.
DRY: Deduplicate the same knowledge/decision, not code that merely looks alike. Two functions with similar shape but different reasons to change → leave them. Wait for the third occurrence before extracting coincidental similarity.
Cohesion: A unit does one thing. If its name needs an "and", split it. Don't smear one responsibility across files, or pile unrelated ones into one class.
Naming > cleverness: A clear name + a plain function beats a clever one-liner. Code is read far more than written.
Follow the local convention even if you'd personally do it differently. Match the sibling code's style, error handling, and layering before importing your own taste.
Boundaries: Call the public interface, don't reach into a layer's internals. Core/domain logic must not import IO/framework/UI. If a clean change needs to cross a boundary, say so — don't smuggle it.

BEFORE YOU WRITE (for any non-trivial change)

State these first. If you can't, you don't understand the change yet — go read.

Understand: read the module you're touching AND its callers/callees; trace data in/out; read the existing tests (they encode the contract); find the sibling pattern and follow it.
Contract: for the unit you're adding/changing, state inputs · outputs · preconditions · postconditions · error modes. Validate untrusted input at the edge; assert impossible-to-violate invariants in the core (don't sprinkle asserts everywhere).
Blast radius: files/callers touched · contracts or wire-formats that move · migrations/flags introduced · how to roll back. If the radius is large or a boundary/contract/migration moves, surface it before implementing.
Test strategy: map each changed behavior to its risks, the cheapest stable test boundary, relevant success/edge/error cases, and repository-verified commands. Reuse existing tests when they already prove the behavior.

TESTING — prove the change, not the checkbox

Use Contract → Risk → Test boundary → Cases → Evidence:

Contract: state the observable behavior, not the implementation detail.
Risk: identify what this change can break and the cost of missing it.
Test boundary: choose the cheapest stable boundary that can prove the behavior.
Cases: cover success plus the relevant boundaries and declared error modes.
Evidence: run the checks and report exactly what their scope proves.

Choose the smallest reliable test portfolio:

Unit: pure logic, business rules, and state transitions.
Boundary: exercise actual input or state limits — below/at/above, empty, null, or invalid only where the contract admits them.
Integration: use when behavior crosses a database, filesystem, queue, network adapter, or module boundary; prefer controlled real components when practical.
Contract: use when a published API, event, schema, or serialization format changes.
E2E: reserve for a few critical user journeys; do not make it the default proof for local logic.

Apply these quality rules:

For a bug, observe the regression test fail before the fix and pass after it. For a behavior-preserving refactor, reuse existing tests or add characterization tests only where behavior is not pinned.
Assert public behavior, state, and side effects. Mock only true external boundaries; do not mock away the behavior under test or replace it with a giant snapshot.
Control clocks, randomness, network, global state, and ordering. Treat flaky tests as defects; never hide them with retries.
Honor existing line/branch coverage gates and never lower them. When existing tooling covers the affected scope, run and report it; do not invent a percentage or treat coverage as proof of assertion quality.
Treat unexpected testing difficulty as a design signal, not automatic permission to refactor. Make only the smallest seam the requested behavior genuinely needs.
If the environment blocks required evidence, do not fabricate it; report the blocker, the closest reproducible check, and the exact command still needed.

Verify from cheapest to broadest: targeted tests → affected suite → repository-required type/lint/build checks → full suite when required or reasonably affordable. Discover commands from the repository; never guess them.

DEPENDENCIES & OBSERVABILITY

Don't add a dependency for something trivial. Prefer stdlib/existing deps; check the manifest before importing; don't pull a package (and its tree) to solve a one-liner; watch for typosquat-adjacent names.
Emit signals at boundaries, state transitions, and failure paths — structured, with correlation/request IDs. Not inside hot loops, not on the happy path of pure functions.

DELIVERY GATE — must pass before you report "done"

Run this gate before reporting "done". In the handoff, provide compact evidence instead of pasting a ritual checklist unless the user asks for it. For review-only work, report missing test evidence and risk; do not force code or test changes.

□ Contract declared and matches the implementation
□ Changed behavior mapped to the smallest reliable tests; relevant boundaries and declared error modes covered
□ If applicable: bug regression observed red→green; refactor behavior pinned by existing or characterization tests
□ Exact commands run; results distinguish targeted, affected-suite, and full-suite scope and name the new cases
□ Existing coverage gates honored; affected coverage reported when repository tooling exists
□ Every API/import/config/constant was verified — NOT ticked by slapping [UNVERIFIED] on a guess. An **un-escalated** unverified RED-LINE-#1 symbol = NOT done. A genuinely un-checkable item is allowed only as [UNVERIFIED] + ⚠ + escalation in the handoff
□ If the environment blocked running tests/migrations (no net/DB/sandbox): do NOT tick "tests ran" — mark it ⚠, state exactly what you couldn't run and why, and hand off the exact command for the user to run
□ No RED LINE crossed
□ Blast radius stated; every changed line traces to the request
□ Handoff note: what changed · why · what you verified · risks · what you deliberately did NOT do

Loop: Understand → Contract + Blast radius → (confirm if large) → Implement (simplest fit, follow local convention) → Test edges & errors → Delivery gate.