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:

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.

No test runner, or tests that do not start, is a 1.4 finding by itself.

Hard gate: line coverage of all src/**/*.ts(x) except tests, vite-env.d.ts, and main.tsx must be ≥ 80%. If the tool was measuring a smaller set, the number does not count.

For each non-trivial file: matching .test.ts / .spec.ts? If not, Tests: ✗. Hooks and services should take dependencies from React context. vi.mock without a comment is a 1.6 finding.

Step 4 — Score the public criteria

Use hunt hits, files you read, coverage scope, and the package audit. Each score needs one or two sentences with paths. N/A only when the criterion does not apply (2.6 with no Raw usage is the usual case). Do not score 1.2 here.

1.1 No known bugs

Check: TODOs on critical paths; missing ErrorBoundary; fetch UI without loading/error/empty; useEffect leaks; null-unsafe access.

Fail this criterion (1–2) if: primary flows crash or silently fail; no error UI on core fetches; data can be corrupted.

Score 3 if: no ErrorBoundary, or several screens missing empty/error states, but core paths still fail visibly rather than corrupt data.

1.3 Dependencies and packages

Check: lockfile, majors behind, CVEs, deprecated, install scripts.

Fail this criterion (1–2) if: high or critical CVE, deprecated production dep, or ≥ 2 majors behind on a production dep with no plan.

1.4 Test coverage

Check: tooling actually runs; what is excluded; line coverage at full src/ scope; tests exist for non-trivial modules.

Fail this criterion (1–2) if any of these are true:

• Coverage tooling missing or tests do not run
• Production files (pages, hooks, components, services, feature modules) are in coverage.exclude / coveragePathIgnorePatterns, or omitted from coverage.include
• Test files are in test.exclude / testPathIgnorePatterns so they never run
• Full-scope line coverage is below 80%

The number Vitest/Jest prints does not override the bullets above.

1.5 Dead code and maintainability

Check: unused exports, unreachable routes, commented-out blocks, lint/tsc, any, unused files (code-quality Steps 1, 8, 9).

Fail this criterion (1–2) if: unreachable pages, unused files, large dead blocks, lint errors, or production any / as any.

1.6 Coding patterns and testability

Check: DI via React context; interface-based services; ViewModel hooks for pages; no new CogniteClient outside bootstrap (code-quality Steps 5–6).

const defaultDependencies = { useDataSource, useAnalytics };
export type UseMyHookContextType = typeof defaultDependencies;
export const UseMyHookContext = createContext<UseMyHookContextType>(defaultDependencies);
export function useMyHook() {
  const { useDataSource } = useContext(UseMyHookContext);
}

Fail this criterion (1–2) if: hooks/services hard-code the SDK or other deps with no injection path, so tests can only use vi.mock; or pages mix fetch + render with no ViewModel on core screens.

2.1 DMS query patterns (search / query vs heavier paths)

Check: Read-heavy UI should use instances.query or instances.search, not default to instances.list.

2.2 Server-side filtering

Check: Filters, limits, and projections in the API request — not download-then-filter in the browser.

Fail this criterion (1–2) if: large or unbounded reads are reduced in memory.

2.3 Limits, pagination, and prefetch

Check: Explicit limits; cursor or page; no prefetch of pages the user may never open.

Fail this criterion (1–2) if: lists are unbounded, or N+1 / deep prefetch.

2.4 Rate of calls — do not hammer DMS

Check: Debounce, batch, cache; no identical requests in a tight loop.

Fail this criterion (1–2) if: request storms against DMS.

2.5 Throttling and 429 responses — backoff with jitter

Check: cdfTaskRunner / QueuedTaskRunner (or equivalent concurrency cap) around CDF calls; on 429, exponential backoff with jitter; respect Retry-After; bounded retries.

Fail this criterion (1–2) if: many parallel CDF calls with neither a concurrency cap nor 429 handling.

Missing QueuedTaskRunner but TanStack Query (or similar) retries with backoff: score 3, not a pass.

2.6 CDF Raw

Check: client.raw, listRows, insertRows, retrieveRow. Unbounded paging; full-table scan then filter; Raw as the primary store.

N/A if unused — do not score this under 2.1.

Fail this criterion (1–2) if: Raw is the system of record, or rows are listed without a bound and filtered in the client.

3.1 Aura design system

Check: Aura components and tokens for layout, forms, tables, feedback, typography.

A score of 3 does not block approval. Missing aria-label / unlabeled inputs → Should Fix even if Aura scores 4–5.

Step 5 — Categorize findings

• Must fix (score 1–2): high/critical CVEs, broken core flows, unbounded API calls, coverage < 80% or coverage/test exclude lists hiding code or tests, unreachable pages or significant dead code, lint errors, production any, no DI path, unbounded or primary-store CDF Raw.
• Should fix (score 3): missing ErrorBoundary, missing loading/error/empty on some screens, missing QueuedTaskRunner when some retry exists, missing tests for non-trivial modules, client-side filtering of large datasets, missing backoff, a11y gaps, vi.mock overuse, oversized components that mix fetch and UI, naming / folder-structure misses from code-quality.
• Nice to fix (score 4 gaps): minor Aura inconsistencies, small cleanup, non-critical package updates, minor dead exports.

Each Must Fix item needs _Impact:_ (one sentence on user/customer consequence).

Scores table every report must include:

Step 6 — Shared verify

The caller may add extra verify steps (artifact paths, no GitHub curl, Codespace, …). These always apply:

1. Every hunt in Step 1 has hits or none (blank is not a result), including the code-quality searches.
2. code-quality was loaded from cognitedata/builder-skills (local or main). Its fix steps were not applied.
3. Coverage config was read. If excludes hide production code or tests, 1.4 is 1–2 and there is a Must Fix item — even if the printed % is ≥ 80%.
4. Every scored criterion (1.1, 1.3–1.6, 2.1–2.6, 3.1) has a score or N/A (2.6 only).
5. Every Must Fix has _Impact:_.
6. Open must/should/nice counts match the lists.