flows-review-checks

name	flows-review-checks
description	Canonical Flows technical-review checks and scoring (hunt commands, coverage scope, package audit, public criteria, Must/Should/Nice). Loaded by flows-code-review and by any external review skill. Do not copy these checks into another skill. Do not use this skill to fix code. Use when an orchestrator says to load flows-review-checks, or when aligning two review flows on the same bar.
allowed-tools	Read, Glob, Grep, Bash, Write

Flows review checks (shared)

This file is the public definition of what a Flows technical review must search for in this repo, how strict each bar is, and how to score 1.1, 1.3–1.6, 2.1–2.6, 3.1.

It does not decide:

• whether you build the app
• Codespace vs local
• Jira / Zendesk / git round folders
• output paths (reviews/code-review/… vs reviews/external-submissions/…)

The caller sets those. Then it reads this file and runs every check.

Do not edit the app. Do not “fix it now.”

Load code-quality from the same cognitedata/builder-skills repo (see hunt 1.5). Run its searches. Ignore every “fix / replace / write the file / pnpm add” instruction in that skill. Other skills (test-coverage, …) stay fixers and are not the review bar.

Caller contract

Before any command in this file:

1. cd to the app root (the directory with package.json and src/).
2. All greps and test/coverage commands run from that directory.
3. The caller names where to write artifacts. This file only defines content.

If a search was not run, that check is not a pass.

What the caller must produce

Whatever the filenames, the review must include:

1. File inventory — every .ts/.tsx except node_modules, dist, .cognite-bundles. Include vitest.config.* / vite.config.* / jest.config.*. Columns: Structure, Quality, Patterns, Tests, Notes. Then read every non-trivial production file (skip barrels, generated types, tests).
2. Findings — every hunt below, with hits or none. Then must / should / nice with file:line.
3. Package audit — Step 2 in this file.
4. Scored report — criteria 1.1, 1.3–1.6, 2.1–2.6, 3.1, must/should/nice lists, _Impact:_ on every Must Fix.

A 1–2 on any criterion is Must Fix. 3 is Should Fix. Gaps at 4 are Nice Fix.

---

Step 1 — Hunt

Run every command. Do not sample src/ and stop.

1.1 Coverage and test config (criterion 1.4) — before you believe any %

Read vitest.config.*, vite.config.* (test / coverage blocks), and jest.config.*. Then:

grep -n -A 40 -E "coverage|exclude|include|coveragePathIgnorePatterns|collectCoverageFrom|testPathIgnorePatterns" vitest.config.ts vitest.config.mts vitest.config.js vite.config.ts vite.config.mts jest.config.ts jest.config.js jest.config.mjs 2>/dev/null

Record two lists:

1. coverage.exclude / coveragePathIgnorePatterns / inverted coverage.include — files not measured.
2. test.exclude / testPathIgnorePatterns — tests that never run.

Allowed coverage excludes: *.test.*, *.spec.*, vite-env.d.ts, src/main.tsx, generated code. Nothing else under src/ — not src/pages/, src/components/, src/hooks/, src/services/, individual feature files, or src/**/*.tsx.

If production files are excluded, the printed coverage number is invalid. Criterion 1.4 scores 1 or 2 even if Vitest prints ≥ 80%. Same if tests themselves are on an exclude list so they do not run.

1.2 Correctness (criterion 1.1)

grep -rn --include="*.ts" --include="*.tsx" -E "ErrorBoundary|componentDidCatch|getDerivedStateFromError" src/
grep -rn --include="*.ts" --include="*.tsx" -E "(TODO|FIXME|HACK|XXX):" src/ | grep -v ".test." | grep -v ".spec."
grep -rn --include="*.tsx" --include="*.ts" -B 2 -A 15 "useEffect" src/
grep -rn --include="*.tsx" -E "useQuery|useMutation|isLoading|isPending|isError" src/

Flag: no ErrorBoundary; fetch UI with no loading / error / empty state; useEffect with timers, listeners, or async work and no cleanup; TODOs on critical paths.

1.3 CDF Raw (criterion 2.6)

grep -rn --include="*.ts" --include="*.tsx" -E "client\.raw\.|\.raw\.(listRows|insertRows|retrieveRow|deleteRows)|listRows|insertRows|retrieveRow" src/

If there are no hits, 2.6 is N/A. Do not fold Raw issues into 2.1.

Flag: Raw as the primary store; paging with no limit/cursor; download-then-filter in the client.

1.4 DMS and limits (criteria 2.1–2.5)

grep -rn --include="*.ts" --include="*.tsx" -E "instances\.(list|search|query|aggregate|retrieve)" src/
grep -rn --include="*.ts" --include="*.tsx" -E "QueuedTaskRunner|cdfTaskRunner" src/
grep -rn --include="*.ts" --include="*.tsx" -E "429|Retry-After|exponential|backoff" src/

Flag: instances.list for read-heavy UI that could be query/search; no limit/cursor; client-side filter of large results; no concurrency cap and no 429 handling.

1.5 Quality and testability (criteria 1.5, 1.6) — includes code-quality

Load skills/code-quality/SKILL.md from this same repo (cognitedata/builder-skills). Local file if the workspace is builder-skills; otherwise:

curl -fsSL https://raw.githubusercontent.com/cognitedata/builder-skills/main/skills/code-quality/SKILL.md

Use it for the bars (150-line components, naming, ViewModel, DI, dead code). Do not apply its fixes. Run every command below (same searches as that skill).

grep -rn --include="*.ts" --include="*.tsx" -E ": any|as any|<any>|as unknown as" src/
grep -rn --include="*.ts" --include="*.tsx" "vi\.mock" src/
grep -rn --include="*.ts" --include="*.tsx" -E "createContext|useContext" src/hooks/ src/contexts/ 2>/dev/null
grep -rn --include="*.ts" --include="*.tsx" -E "^import.*from\s+['\"]\.\./" src/hooks/
grep -rn --include="*.ts" --include="*.tsx" -E "new CogniteClient|createCogniteClient" src/
grep -rn --include="*.ts" --include="*.tsx" -E "class\s+\w+(Service|Client|Repository|Manager)" src/
grep -rn --include="*.tsx" --include="*.ts" -l "useQuery\|useMutation\|sdk\.\|client\." src/pages/ src/views/ 2>/dev/null
grep -rn --include="*.ts" --include="*.tsx" -l "ViewModel" src/hooks/ 2>/dev/null
grep -rn --include="*.tsx" --include="*.ts" -E "console\.(log|debug)" src/
grep -rn --include="*.tsx" --include="*.ts" -E "path:\s*['\"]|<Route" src/
pnpm run lint 2>/dev/null || npm run lint 2>/dev/null || true
pnpm exec tsc --noEmit 2>/dev/null || npx tsc --noEmit 2>/dev/null || true

Component size (flag .tsx over 150 lines, then read — mixed fetch + render is the problem, not length alone):

node -e "const fs=require('fs'),path=require('path');function walk(d){return fs.readdirSync(d,{withFileTypes:true}).flatMap(e=>{const p=path.join(d,e.name);return e.isDirectory()?walk(p):p.endsWith('.tsx')?[p]:[]})}walk('src').map(p=>({p,l:fs.readFileSync(p,'utf8').split('\n').length})).sort((a,b)=>b.l-a.l).forEach(({l,p})=>console.log(l,p))"

Possibly unused production files:

for file in $(find src -name "*.ts" -o -name "*.tsx" | grep -v ".test." | grep -v ".spec." | grep -v "node_modules"); do
  basename=$(basename "$file" | sed 's/\.[^.]*$//')
  imports=$(grep -rn --include="*.ts" --include="*.tsx" "$basename" src/ | grep -v "$file" | wc -l)
  if [ "$imports" -eq 0 ]; then echo "UNUSED: $file"; fi
done

Flag: any / as unknown as in production; lint or tsc errors; new CogniteClient outside bootstrap; hooks that import deps instead of context; pages with useQuery / SDK and no ViewModel; vi.mock with no comment; console.log/debug; unused files; unreachable routes; components over 150 lines that mix data fetching with UI.

Per code-quality: lint errors and production any are blocking (Must Fix). Unreachable pages / unused files / large dead blocks are Must Fix (already 1.5).

Findings shape

# Findings: [app name]

## Config inspected
- Coverage config file(s): …
- Production paths excluded from coverage: … (or none)
- Tests excluded from the test run: … (or none)

## Searches
| Check | Hits (file:line or none) |
| ----- | ------------------------ |
| ErrorBoundary | |
| coverage/test exclude | |
| CDF Raw | |
| instances.list/query/search | |
| QueuedTaskRunner / 429 | |
| any / vi.mock | |
| lint / tsc | |
| CogniteClient / DI / ViewModel | |
| unused files / console.log | |
| components > 150 lines | |

## Must / should / nice
- [ ] … — file:line — criterion

Every later score must point at this hunt.

---

Step 2 — Packages

Two commands — do not loop npm view per package:

npm outdated --json 2>/dev/null || true
npm audit --json 2>/dev/null || true

From npm outdated --json: packages absent 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 advisories. High or critical CVEs are Must Fix (1.3 scores 1–2).

Spot-check npm view <pkg> deprecated only for packages that are already flagged (major behind, in audit, or an unfamiliar name).

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).

Table shape:

## Package audit: [app name]

### Dependencies

| Package | Used version | Latest | Deprecated | CVEs | Health |
| ------- | ------------ | ------ | ---------- | ---- | ------ |

### Security audit

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

#### Vulnerabilities

| Package | Severity | Title | Patched in | Advisory |
| ------- | -------- | ----- | ---------- | -------- |

---

Step 3 — Test coverage

Only after Step 1.1. If production files were excluded, say so before quoting Vitest’s number, and score 1.4 as 1–2.

npx vitest run --coverage
# or: npx jest --coverage
# or: npm test -- --coverage

Record framework, pass/fail/skip counts, statement/branch/function/line percentages.