pine-review-code

name	pine-review-code
description	Structured code review for Pine design-system components. Reviews Stencil decorators (@Component, @Prop, @Event, @Method), TypeScript types, JSDoc consistency across components, lifecycle methods, spec + e2e test coverage, and auto-generated file hygiene. Categorizes issues by severity.

Review Code (Pine)

Review code changes against Pine project standards. Identify pattern violations, anti-patterns, and engineering issues in Stencil web components. Provide structured, actionable feedback.

When to Use

• After implementing a new component, prop, event, method, or behavior
• Before opening a PR against main
• When asked to review specific files or changes

Review Process

1. Identify changes — git diff origin/main...HEAD --name-only
2. Run automated checks for changed components:

   npx nx run @pine-ds/core:lint
   npx nx run @pine-ds/core:test
   

   Or, scoped to a single component, prefer building first:

   npx nx run @pine-ds/core:build  # regenerates readme.md + components.d.ts
   

   Any failures are automatic BLOCKERs.
3. Check pattern compliance per area (see Review Criteria below).
4. Output structured review using the Output Format below.

Review Criteria

Stencil Components — BLOCKER

• Missing @Component decorator config:
  • Every *.tsx component class needs tag: 'pds-<name>' and styleUrls: [...] (tokens first, then component styles).
  • Shadow DOM (shadow: true) — Pine's convention; non-shadow needs justification.
• @Prop without explicit type — @Prop() x; with no type annotation is a BLOCKER. Always declare string | number | boolean | <UnionType> with a default where applicable.
• Public @Prop / @Event / @Method without JSDoc — Public API surface must be documented. JSDoc on these decorators feeds Storybook docs and consumer-facing TypeScript types.
• JSDoc terminology inconsistent with sibling components — Per Pine CLAUDE.md: "When adding a prop or event that exists in a similar form on another component, match the wording and phrasing of the existing description." componentId, disabled, size, variant, etc. should read identically across components.
• Breaking change without BREAKING CHANGE: footer — Removing or renaming a public @Prop, @Event, @Method, or changing a default value without a BREAKING CHANGE: block in the commit body. Conventional Commits drives semantic version bumps.
• Hand-edits to auto-generated files — readme.md and components.d.ts are regenerated by Stencil's build. Any manual edit is a BLOCKER; the fix is to rebuild and let Stencil regenerate.
• Failing tests / lint — Stencil spec, e2e, ESLint, Prettier failures count as BLOCKERs.
• @Event without EventEmitter type — @Event() onClick; with no generic is a BLOCKER. Use @Event() onClick: EventEmitter<MouseEvent>; (or whatever the payload type is, including void).
• @Method not async — Stencil requires public methods to be async. Sync @Method() declarations are a BLOCKER.

Stencil Components — SHOULD FIX

• Missing spec test (*.spec.tsx) — Every component must have at least one spec covering its primary render path. Every new prop / event / variant adds at least one assertion.
• Missing e2e test (*.e2e.ts) when the change touches event handlers, focus management, or async behavior.
• Storybook story not updated for new prop / variant — Stories live in <component>/stories/. New props need argTypes and at least one exposed variant.
• @State used where a derived computation would do — Don't store what you can derive in a getter.
• onClick etc. arrow function defined in render() body — Allocates a new function each render; hoist to a class field or method.
• Mutating props — Props are read-only inputs. Mutation breaks Stencil's change-detection contract.
• Lifecycle method misuse — componentWillLoad is async-safe, componentDidLoad runs once after first render. Don't move work that belongs in componentWillLoad into componentDidLoad or vice versa.
• TypeScript any — Use a union type, generic, or unknown with a narrowing check instead.

Stencil Components — CONSIDER

• Shared types not in @utils/types — Reusable type aliases (ChipSizeType, etc.) should live in shared utility types rather than inline in a single component.
• Inline event payload types — Hoisting them to a named type improves consumer-side TypeScript and docs.
• @Watch use — When a prop change should trigger a side-effect, @Watch is preferred over imperative componentDidUpdate inspection.
• Sub-component file layout — Composite components (e.g. pds-table/pds-table-row/) should nest sub-components inside the parent directory.

React Wrappers — SHOULD FIX

• Edits to libs/react/src/ that are not regeneration outputs — React wrappers are generated by Stencil's react-output-target. Authoring a wrapper by hand bypasses the generator and will be overwritten on the next build.

Conventional Commits — SHOULD FIX

• Scope mismatch — Commit scope must match the component name: feat(pds-table):, fix(pds-button):, style(pds-chip):, test(pds-input):, docs(pds-modal):, refactor(pds-form):. Tooling-only changes use chore: with no scope.
• Ticket / issue number in branch name or commit — Per Pine CLAUDE.md: "Do NOT include ticket or issue numbers in branch names or commit messages."
• Multi-line commit subject — Pine prefers single-line commit subjects; details go in the body.

Severity Definitions

Level	Meaning	Action
BLOCKER	Breaks Stencil contract, breaks public API without footer, edits generated files, failing tests/lint	Must fix
SHOULD FIX	Violates Pine patterns or best practices	Fix unless good reason
CONSIDER	Could be improved but acceptable	Optional

Output Format

Number items sequentially across all sections — do not restart numbering in each section. Section headers still show per-section counts.

## Pine Code Review

**Files Reviewed:** [list]
**Components Affected:** [pds-<names>]
**Overall Assessment:** APPROVED | NEEDS CHANGES | BLOCKER

---

### BLOCKERS ([count])

#### 1. [Issue Title]
- **File:** `path/to/file:line`
- **Issue:** [description]
- **Fix:** [specific action]

---

### SHOULD FIX ([count])

#### 2. [Issue Title]
- **File:** `path/to/file:line`
- **Issue:** [description]
- **Fix:** [specific action]

---

### CONSIDER ([count])

#### 3. [Issue Title]
- **File:** `path/to/file:line`
- **Suggestion:** [description]

---

### What's Good
- [Positive observation]
- [Pattern followed correctly]

Anti-Patterns in Reviewing

• Do NOT nitpick formatting — ESLint / Stylelint / Prettier handle that.
• Do NOT review dist/ or other generated directories.
• Do NOT require a story for a spec-only or doc-only change.
• Do NOT flag a Stencil pattern just because it differs from a React- class equivalent; Stencil has its own contract.
• Do NOT suggest comment additions to self-explanatory decorator code; JSDoc on the decorator is enough.