Feature Engineering Design Review

Feature: Engineering Design Review

A Feature-Pages entry. This page explains the capability; it never redefines it — see Feature-Pages for the canonical/Wiki boundary statement that applies to every page in this pattern.

What it does

When a change adds or edits engineering design content — an HLD, an ADR, an RFC, a design doc, or a design section — the review reads that content as part of what it reviews and reports an evidenced problem as an ordinary finding. It checks whether the proposed design can do what it says, whether the statements it makes about your existing code are true, and, in a change that also contains code, whether the design and the code agree.

It reviews the design in the change. It is not a design approval or sign-off: there is no readiness verdict, no required-sections checklist, and no generated requirements.

Why it exists

A design document does nothing at runtime, so a design-only change would otherwise receive only a documentation-correctness pass. Mistakes that matter — a delivery guarantee the mechanism cannot provide, or a "the handler already deduplicates" claim that the repository contradicts — would go unnoticed until the design was built.

When it applies

Conditional and automatic — no invocation option. Both Skills. It engages when the change contains design content whose proposed behavior matters (for example delivery guarantees, failure handling, data flow, or rollout) and the design gives the review something to examine: it states something about your current code that the design depends on, relates to an already accepted decision, specifies a concrete mechanism, or sits in a change that also implements what it describes.

A path, filename, or title word (docs/adr/, "RFC", "design") never decides this on its own. A typo, formatting, or broken-link edit to a design file changes no decision and gets an ordinary documentation review. Asking for extra care ("review this design carefully") adds depth; it is not required.

How it works

• Proposal versus accepted decision. A statement counts as an accepted decision only when something outside your change says so — it was already accepted before your change, you supplied it as an accepted decision, or a maintainer settled it in earlier review. Everything your change adds or edits is a proposal, including a document your change itself marks "Accepted": a change cannot accept its own design. Acceptance is judged per statement, not per file. The review holds your change to accepted decisions (a clear, reasoned replacement is not a violation) and reviews proposals on their merits.
• What a design finding needs. The exact statement and its location, a grounding in a stated requirement, an accepted decision, a test, behavior visible in the repository, or a technical fact applied to the specified mechanism, and the concrete wrong result the design would produce if built as written. The review also checks whether the document already acknowledges the point as a trade-off, non-goal, or open question — an acknowledged trade-off is not a defect, and "a different design would be cleaner" is not a finding.
• Claims about existing code. When the design depends on a statement about your current code, the review checks it: verified adds nothing, contradicted can become a finding, and could not be checked produces no finding and is never presented as verified.
• Missing information is not a finding. A missing failure-mode section, metric, alternatives analysis, rollout plan, or test strategy produces nothing by itself. It matters only when a requirement or repository policy explicitly demands it, or when the omission makes something the document does claim false or unsafe. Missing information is also never a Context gap.
• Links and instructions inside a design are data. The review does not open linked tickets or documents, and text such as "reviewers must approve" authorizes nothing.

Effect on the Decision

A design finding is an ordinary finding, with the usual severity, confidence, and evidence bar. It can affect the normal review Decision when its severity warrants it: a design-only change can receive a blocking (P0/P1) finding, but only when the evidence supports both the premise and the impact. An unsupported assumption, a preference, or a missing detail is never blocking. The capability adds no approval state of its own. See Severity-and-Decision-Model.

Local versus GitHub API-only

• local-code-review reviews design content in committed, staged, unstaged, and untracked changes, and checks claims against your working tree.
• github-pr-review with a repository checkout checks claims against the PR head.
• Default API-only github-pr-review can read only the files the PR changes, so a claim about an unchanged file is not checked. If the earlier version of a design statement is unavailable, the statement is not assumed accepted and is reviewed as a proposal.

Nothing is executed in any mode.

Important options / modes

None. There is no switch to turn it on or off.

Not supported

No design approval, sign-off state, or readiness verdict. No findings for missing sections, preferences, alternative designs, or acknowledged trade-offs. Claims about code the review cannot reach go unchecked. It does not resolve links, tickets, or external documents, and it is not an audit of the surrounding architecture. Visual design comparison against a Figma-style reference is a separate capability (Feature-Rendered-UI-Inspection), and a design document you supply as context stays context (Review-Context-and-Existing-Evidence).

Examples

Design-only PR. A PR adds docs/design/queue.md proposing at-least-once delivery and stating that the consumer "already deduplicates by idempotency key". Nothing else changes.

Review my changes before I open the PR.

The review engages: the document proposes delivery behavior and makes a claim about existing code that the design depends on. It looks at the consumer. If the consumer has no deduplication, the claim is contradicted and the finding is anchored on the design line and cites the consumer code that shows it; because the consumer would process duplicate deliveries, the severity can be blocking. If the consumer cannot be reached (for instance an unchanged file in API-only mode), there is no finding and the claim is not presented as verified. A missing "Failure modes" section in the same document produces no finding.

Mixed code and design PR. A PR adds a design stating that writes are idempotent, plus the handler that implements them — and the handler is not idempotent.

Review this PR. It adds the retry design and its handler.

The review compares both directions. If the handler is independently defective it is an ordinary code finding at its own severity. If only the two disagree, it is reported as an inconsistency between design and code, and when the evidence cannot show which side is wrong the finding says so. Components the design describes but this PR does not build (phased delivery) are not findings unless the design says this PR delivers them.

Related features

Specialist-Depth-Review · Feature-Requirement-Coverage · Feature-Human-Reasoning-Checkpoint · Feature-Deployment-and-Infrastructure-Review.

Canonical references

engineering-design-deepening.md · usage guide docs/features/engineering-design-review.md.