flows-code-review

What this skill does

# Flows Code Review

This skill is the **technical review** step of the Flows app certification flow:

```
flows-app-brief  →  build  →  flows-code-review (this skill, repeat until clean)  →  flows-design-review  →  flows-external-app-submit
```

## Pre-checks

Before starting, confirm:
- `package.json` exists (Node/TS project)
- We are inside a git repository (`git rev-parse --git-dir`)
- If `App-Brief.md` is missing at repo root, warn the user that `flows-app-brief` should be run first, but continue.

Determine the feedback round: look at `reviews/code-review/`. If it doesn't exist, use `feedback-round-1/`. Otherwise increment to the next missing round number.

## Step 1 — Load review guidance

Before any scoring, read all relevant skill files. The skills ship alongside this skill in the same repo and are available locally. For each skill below, `Glob '**/skills/<name>/SKILL.md'` and `Read` the first match.

**Required reads (always):**

| Skill | Informs criteria |
| --- | --- |
| `correctness-and-error-handling` | 1.1 (bugs), 1.4 (tests), plus error-state patterns |
| `security` | 1.2 (CDF via SDK), 1.3 (dependencies, CVEs) |
| `dependencies-audit` | 1.3 (packages audit) |
| `test-coverage` | 1.4 (coverage gate), 1.6 (testability) |
| `code-quality` | 1.5 (dead code), 1.6 (patterns) |
| `dm-limits-and-best-practices` | 2.1–2.5 (DMS query patterns, limits, rate) |
| `performance` | 2.3–2.4 (pagination, call rate) |
| `design` | 3.1 (Aura consistency) |

**Conditional read:**
- `setup-flows-auth` — read if the app imports `connectToHostApp`, `useHostApp`, or configures OAuth in Vite.

**Note on DMS patterns:** the `dm-limits-and-best-practices` skill is the canonical local reference for search vs joins, cardinality, and query-time semantics — no external `semantic-knowledge/` submodule is needed.

Do not proceed to scoring until all skill reads are complete.

## Step 2 — Build the file inventory (`review-files.md`)

List **all `.ts` and `.tsx` files** in the app (exclude `node_modules`, `dist`, `.cognite-bundles`). For each file assess:

- **Structure** (1–5): file size, single responsibility, clear organization
- **Quality** (1–5): type safety, no `any`/`unknown` casts, proper error handling, no dead code, clean naming
- **Patterns** (1–5 or N/A): DI/testability, correct platform patterns, no anti-patterns; `N/A` for pure utils/types
- **Tests**: `✓` adequate | `⚠` has tests with meaningful gaps | `✗` no test file | `N/A` (presentational-only or barrel)

Write to `reviews/code-review/feedback-round-<N>/review-files.md`:

```markdown
## File inventory: [app name]

| File | Structure | Quality | Patterns | Tests | Notes |
| ---- | --------- | ------- | -------- | ----- | ----- |
| src/main.tsx | 5 | 5 | N/A | N/A | Clean entry point |
| src/services/cdfHelper.ts | 3 | 2 | 2 | ✗ | No DI; raw sdk.get/post; no test file (1.2, 1.6, 2.5) |
| ... | ... | ... | ... | ... | ... |
```

## Step 3 — Build the package inventory (`review-packages.md`)

Gather dependency state with two one-shot commands — do **not** loop per package (`npm view <pkg>` is one network call per package; 40–80 calls = 2–4 minutes):

```bash
npm outdated --json 2>/dev/null || true   # exits 1 when outdated packages exist — normal; parse stdout regardless
npm audit --json 2>/dev/null || true       # exits 1 when vulnerabilities exist — parse stdout regardless
```

From `npm outdated --json`: packages absent from the output are up-to-date; packages present show `current` / `wanted` / `latest`. Flag any in `dependencies` (not `devDependencies`) that are ≥ 1 major behind.

From `npm audit --json`: parse severity counts and per-package advisory details.

**Deprecated status:** `npm outdated` does not report this. Only spot-check with `npm view <pkg> deprecated` for packages that already triggered another flag (≥ 1 major behind, in audit advisories, or unfamiliar name).

Assign health: **Pass** (up-to-date or ≤ 1 minor behind, 0 critical/high CVEs) | **Warn** (1 major behind in `dependencies`, or moderate CVE) | **Fail** (≥ 2 majors behind, or high/critical CVE, or deprecated).

Write to `reviews/code-review/feedback-round-<N>/review-packages.md`:

```markdown
## Package audit: [app name]

### Dependencies

| Package | Used version | Latest | Deprecated | CVEs | Health |
| ------- | ------------ | ------ | ---------- | ---- | ------ |
| react | ^18.2.0 | 18.3.1 | No | 0 | Pass |
| some-old-lib | ^1.0.0 | 1.0.3 | No | 0 | Fail |
| ... | ... | ... | ... | ... | ... |

### Security audit

| Severity | Count |
| -------- | ----- |
| Critical | 0 |
| High | 0 |
| Moderate | 0 |
| Low | 0 |

#### Vulnerabilities

| Package | Severity | Title | Patched in | Advisory |
| ------- | -------- | ----- | ---------- | -------- |
| example-pkg | High | Prototype pollution | >=2.1.0 | https://github.com/advisories/GHSA-xxxx |
```

## Step 4 — Assess test coverage

Run the test suite with coverage:

```bash
npx vitest run --coverage   # Vite/Vitest projects
npx jest --coverage          # Jest projects
npm test -- --coverage       # generic fallback
```

Record the output: framework, pass/fail counts, statement/branch/function/line percentages. If the test suite fails to run or no framework is configured, note explicitly — **absence of a working test setup is itself a finding for criterion 1.4**.

**Hard gate:** Overall line coverage must be **≥ 80%** for the app to be approved. Coverage applies to all `.ts`/`.tsx` files under `src/` (excluding test files, type declarations, and `main.tsx`). Apps that exclude production files from coverage measurement must be re-measured at full scope. Coverage exclusions hiding untested code are themselves a finding.

Also note per each non-trivial file: is there a corresponding `.test.ts` / `.spec.ts`? If not, mark `Tests: ✗` in the file inventory. Assess whether hooks and services use **dependency injection via React context** rather than direct imports (`vi.mock` usage is a red flag unless justified with a comment).

## Step 5 — Score the 12 criteria

Evaluate all 12 criteria using the rubric below. For each, state the score and one or two sentences of evidence (file paths, patterns observed, or gaps). Mark **N/A** with explanation if a criterion is not applicable.

### 1.1 No known bugs

**Check:** Open issues, obvious regressions, error paths untested, or `TODO: fix` in critical paths.

| Score | Why |
| ----- | --- |
| 5 | No known defects; edge cases handled or explicitly out of scope with safe failure modes. |
| 4 | Minor issues only; none affect core user flows or data integrity. |
| 3 | Some known bugs or rough edges; workarounds exist or impact is limited. |
| 2 | Material bugs, unreliable flows, or silent failures; users or data could be harmed. |
| 1 | Broken primary flows, data corruption risk, or security-adjacent defects. |

### 1.2 CDF access via Cognite SDK only

**Check:** All traffic to CDF goes through the official Cognite SDK. Flag any `fetch`/`axios`/raw REST to CDF-like URLs that bypasses the SDK.

| Score | Why |
| ----- | --- |
| 5 | CDF usage is exclusively via the SDK; non-CDF calls are minimal and intentional. |
| 4 | SDK used for CDF; one borderline or legacy call worth confirming. |
| 3 | Mix of SDK and direct calls with weak justification. |
| 2 | Repeated or critical paths use raw CDF HTTP instead of the SDK. |
| 1 | Custom CDF clients, token handling outside SDK patterns, or undisclosed CDF endpoints. |

### 1.3 Dependencies and packages

**Check:** `package.json` / lockfiles; unmaintained packages, duplicate majors, known CVEs, license issues, install scripts.

| Score | Why |
| ----- | --- |
| 5 | Dependencies are current, trustworthy, and scanned; no red flags. |
| 4 | Small updates or pinning tweaks needed; no serious supply-chain signals. |
| 3 | Outdated or heavy deps; plan to upgrade documented. |
| 2 | High-risk packages, excessive bundle, or unclear provenance. |
| 1 | Known-vulnerable, deprecated, or malicious-adjacent dependenc