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/…vsreviews/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:
cdto the app root (the directory withpackage.jsonandsrc/).- All greps and test/coverage commands run from that directory.
- 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:
- File inventory — every
.ts/.tsxexceptnode_modules,dist,.cognite-bundles. Includevitest.config.*/vite.config.*/jest.config.*. Columns: Structure, Quality, Patterns, Tests, Notes. Then read every non-trivial production file (skip barrels, generated types, tests). - Findings — every hunt below, with hits or
none. Then must / should / nice withfile:line. - Package audit — Step 2 in this file.
- 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:
coverage.exclude/coveragePathIgnorePatterns/ invertedcoverage.include— files not measured.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.
| 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 or data corruption risk. |
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.
| 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 dependency usage. |
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 fromcoverage.include - Test files are in
test.exclude/testPathIgnorePatternsso they never run - Full-scope line coverage is below 80%
The number Vitest/Jest prints does not override the bullets above.
| Score | Why |
|---|---|
| 5 | Critical paths well covered; honest ≥ 80% line coverage; failures easy to localize. |
| 4 | Good honest coverage (≥ 80%) with a few gaps in secondary modules. |
| 3 | Core flows partially tested; honest coverage below 80% or important branches missing. |
| 2 | Sparse tests; coverage well below 80%; or coverage looks high because files were excluded. |
| 1 | No meaningful automated tests, tooling not configured, or most of the app excluded from measurement. |
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.
| Score | Why |
|---|---|
| 5 | Codebase is lean; dead paths removed or clearly feature-flagged with owners. |
| 4 | Minor cruft; quick cleanup possible. |
| 3 | Noticeable dead code or duplication; increases review and bug surface. |
| 2 | Large unused areas or confusing structure; hard to reason about behavior. |
| 1 | Unmaintainable tangle; dead code masks real execution paths. |
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.
| Score | Why |
|---|---|
| 5 | DI via context throughout; interface-based services; ViewModel hooks for pages; type-safe mocks. |
| 4 | Mostly good patterns; one or two hooks import directly but still testable. |
| 3 | Mixed: some injectable, others import directly; vi.mock used frequently. |
| 2 | Most hooks hard-code dependencies; tests rely on module mocking throughout. |
| 1 | No DI; global singletons or direct SDK calls throughout. |
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.
| Score | Why |
|---|---|
| 5 | Read paths use the lightest suitable DMS API; writes and exceptions are documented. |
| 4 | Mostly correct; one or two calls could move to query/search. |
| 3 | Mix of appropriate and heavy reads; performance risk under load. |
| 2 | Default pattern is heavier than needed; likely to stress Postgres or DMS. |
| 1 | Systematic misuse of APIs contrary to platform guidance. |
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.
| Score | Why |
|---|---|
| 5 | Payloads are tight; filtering expressed in queries; pagination is real, not simulated. |
| 4 | Minor over-fetch; easy to tighten. |
| 3 | Repeated patterns of client-side filtering on medium result sets. |
| 2 | Large downloads with in-memory filtering; obvious scalability issue. |
| 1 | Unbounded or near-unbounded reads with client-side reduction. |
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.
| Score | Why |
|---|---|
| 5 | Limits and paging match UX; no wasteful speculative loading. |
| 4 | Reasonable; small tuning opportunities. |
| 3 | Occasional high limits or redundant page fetches. |
| 2 | Frequent large pages or prefetch storms. |
| 1 | Unbounded lists, deep prefetch chains, or N+1 DMS patterns. |
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.
| Score | Why |
|---|---|
| 5 | Request rate matches user intent; caching/dedup in place. |
| 4 | Mostly fine; a hot path could batch or debounce slightly. |
| 3 | Chatty UI or polling without backoff; risk under concurrent users. |
| 2 | Clear risk of overwhelming DMS or shared quotas. |
| 1 | Tight loops, runaway polling, or duplicate parallel identical calls. |
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.
| Score | Why |
|---|---|
| 5 | Backoff + jitter implemented; retries bounded; behavior degrades gracefully. |
| 4 | Backoff exists; jitter or caps could be improved. |
| 3 | Naive fixed-interval retries or missing handling for throttling. |
| 2 | Aggressive retries on 429; thundering herd risk. |
| 1 | No handling; infinite or immediate tight retries on errors. |
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.
| Score | Why |
|---|---|
| 5 | Raw unused, or used for a narrow documented case with limits and server-side filter. |
| 4 | Legitimate Raw use; small tightening (limit/cursor) still needed. |
| 3 | Raw used more than needed; risk under growth. |
| 2 | Unbounded Raw reads or client-side filter of a table. |
| 1 | Raw as primary store, or scan-the-database patterns. |
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.
| Score | Why |
|---|---|
| 5 | Aura used consistently; custom pieces match design language. |
| 4 | Mostly Aura; isolated custom widgets with acceptable alignment. |
| 3 | Mix of ad-hoc UI and Aura; some inconsistency for users. |
| 2 | Largely non-Aura; visually disconnected from platform. |
| 1 | Clashing patterns, inaccessible controls, or no alignment with Aura when feasible. |
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
QueuedTaskRunnerwhen some retry exists, missing tests for non-trivial modules, client-side filtering of large datasets, missing backoff, a11y gaps,vi.mockoveruse, oversized components that mix fetch and UI, naming / folder-structure misses fromcode-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:
| Area | Criterion | Score | Notes |
|---|---|---|---|
| User & customer | 1.1 Known bugs | /5 | |
| User & customer | 1.3 Packages | /5 | |
| User & customer | 1.4 Tests & coverage | /5 | |
| User & customer | 1.5 Dead code | /5 | |
| User & customer | 1.6 Patterns & testability | /5 | |
| Cognite services | 2.1 DMS query patterns | /5 | |
| Cognite services | 2.2 Server-side filter | /5 | |
| Cognite services | 2.3 Limits & pages | /5 | |
| Cognite services | 2.4 Call rate | /5 | |
| Cognite services | 2.5 429 backoff | /5 | |
| Cognite services | 2.6 CDF Raw | /5 or N/A | |
| Brand | 3.1 Aura | /5 |
Step 6 — Shared verify
The caller may add extra verify steps (artifact paths, no GitHub curl, Codespace, …). These always apply:
- Every hunt in Step 1 has hits or
none(blank is not a result), including thecode-qualitysearches. code-qualitywas loaded fromcognitedata/builder-skills(local ormain). Its fix steps were not applied.- 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%.
- Every scored criterion (1.1, 1.3–1.6, 2.1–2.6, 3.1) has a score or N/A (2.6 only).
- Every Must Fix has
_Impact:_. - Open must/should/nice counts match the lists.