flows-review-checks

# 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: ```bash 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) ```bash 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) ```bash 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) ```bash 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: ```bash 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). ```bash 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): ```bash 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: ```bash 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 ```markdown # 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: ```bash 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: ```markdown ## 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. ```bash npx vitest run --coverage # or: npx jest --coverage # or: npm test -- --coverage ``` Record framework, pass/fail/skip counts, statement/branch/function/line percentages.