code-quality
MUST be used whenever reviewing a Flows app for code quality, maintainability, or clean code issues — before a PR review, after a feature is complete, or when the user asks for a code review. Do NOT skip linting steps. Triggers: code quality, code review, clean code, refactor, maintainability, techn
By cognitedata · 1,944 installs
npx skills add cognitedata/builder-skills --skill code-quality
Source repository · Upstream listing
Code Quality Review
Review $ARGUMENTS (or the whole app if no argument is given) for code quality issues. Work through every step below in order and report all findings with file paths and line numbers.
Step 1 — Run the linter first
Before reading any code manually, get a baseline from the automated tools:
List every error and warning. Fix all errors before proceeding — lint errors are not negotiable. Warnings should be reviewed and resolved unless there is a documented exception.
Also run the TypeScript compiler in strict mode to surface any hidden type issues:
List every type error. These must be fixed.
Step 2 — TypeScript type safety
2a — Eliminate any types
Search for any usage across the codebase:
For each hit, replace with the correct type. Common substitutions:
Instead of Use
any for unknown external data unknown + type guard or Zod parse
any for event handlers React.ChangeEvent<HTMLInputElement , React.MouseEvent , etc.
any for CDF responses The SDK's own response types (import from @cognite/sdk )
any[] for arrays T[] with the correct generic
as any casts Proper type narrowing or explicit overloaded function signature
The goal is zero any in src/ . If a third party library forces it, wrap the call in a typed adapter function so any does not leak into the app.
2b — Make impossible states unrepresentable
Use the type system to make invalid states fail at compile time. Fewer reachable states = easier code to read and change.
Branded types — brand primitives so they can't be mixed up. Validate once at the boundary; downstream code trusts the type.
If the project uses a library with native branded type support (e.g. Effect), use their primitives instead of rolling your own.
Discriminated unions over flag bags — replace boolean/optional combos with an exhaustive union:
Search for flag bag patterns:
Flag every type that combines boolean flags where only certain combos are valid. These should be discriminated unions.
2c — Let types flow end to end
DB schema → server → client should share types without manual duplication. Don't restate types you can derive — reach for Pick , Omit , Parameters , ReturnType , Awaited , typeof before writing a new interface.
Flag interfaces that manually restate fields already present on an SDK or DB type — these should use Pick / Omit instead.
2d — Pass objects, not positional arguments
Functions with two or more parameters of the same primitive type should receive a named property object so callers can't silently swap arguments.
Step 3 — Check component size and single responsibility
List all .tsx files with their line counts:
Flag every component file over 150 lines . For each, read it and check:
Does it do more than one thing? (fetch data AND render UI AND handle form state)
Can the fetch logic move to a custom hook ( useAssetData )?
Can sub sections be extracted as named sub components?
Apply the split only when it creates a genuinely cleaner separation — do not split for the sake of line count alone. A well named 200 line component is better than three poorly named 60 line ones.
Step 4 — Find and remove duplicate logic (DRY)
Search for copy pasted patterns across hooks, utilities, and components:
For each set of duplicates:
Extract to src/utils/ if it is a pure function
Extract to src/hooks/ if it contains React state or effects
Extract to a shared component if it is JSX
Step 5 — Enforce dependency injection for external calls
Components and hooks must not import the CDF client directly. The SDK client must be obtained from context (via useCogniteClient() or a prop) so the component is testable in isolation.
Flag any direct client construction outside of the app's bootstrap / auth setup file. The pattern should always be:
Similarly, Atlas tools should receive their dependencies via execute 's closure over a hook provided ref, not by importing a global singleton.
Step 6 — Verify coding patterns and testability
Check that the codebase follows the three core patterns required by the Flows app review process. These patterns keep code testable, maintainable, and consistent.
6a — Dependency injection via React context
Hooks must declare their dependencies through a context type and consume them via useContext , not by importing them directly. This enables testing without module level mocks.
The preferred pattern:
For non React code (utilities, services), use factory functions with partial dependency overrides :
Flag every hook that imports dependencies directly instead of receiving them through context. These are testability concerns even if tests exist today.
6b — Interface based services
Service classes must implement explicit TypeScript interfaces. This keeps production code substitutable and test doubles type safe.
Flag:
Service classes that do not implement an explicit interface
as unknown as T casts in either production or test code — this signals poor interface design
6c — ViewModel pattern
Page level hooks ( useSomethingViewModel ) must separate business logic from presentation. UI components receive data and callbacks only; they contain no data fetching, side effect logic, or direct SDK calls.
Flag:
Page components that contain useQuery , useMutation , or direct SDK calls — this logic should be in a ViewModel hook
Missing ViewModel hooks for pages with non trivial data logic
6d — Test mock quality
Flag:
vi.mock usage without a justification comment explaining why context injection was not possible
as unknown as T casts in test files — signals poor interface design in the production code
Step 7 — Check naming conventions
Read a representative sample of files and verify:
Artifact Convention Examples
Files & directories kebab case asset panel.tsx , use asset data.ts
React components PascalCase AssetPanel , NavigationBar
Variables, functions, hooks camelCase isLoading , fetchAssets , useAssetData
Constants (module level) SCREAMING SNAKE CASE MAX ITEMS , AGENT EXTERNAL ID
TypeScript types & interfaces PascalCase AssetNode , ChartConfig
Boolean variables Auxiliary verb prefix isLoading , hasError , canEdit
Search for common violations:
Step 8 — Remove dead code
Search for unreachable pages (routes defined in the router but whose component is never imported or rendered) and entirely unused files:
Rules:
console.log and console.debug must be removed before shipping (use proper error logging for console.error ).
Commented out code blocks must be removed — version control preserves history.
TODO and FIXME comments older than the current sprint should be resolved or converted to tracked issues.
Unused imports are caught by the linter (Step 1); confirm they are gone.
Hard gate: Unreachable pages, entirely unused files, and significant dead code blocks must be removed before approval. These are blocking findings.
Step 9 — Verify file and export structure
Every feature area should follow a consistent structure. Check that the app's layout matches this pattern:
Flag:
Business logic sitting directly in page components (should be in hooks)
Utility functions living inside component files (should be in utils/ )
Types defined inline in component files when they are used across multiple files (should be in types/ )
Missing index.ts barrel files for component directories (makes imports verbose)
Step 10 — Report findings
Produce a structured report grouped by category:
Category File Line Issue Recommendation
TypeScript src/hooks/useData.ts 18 response as any cast Import and use NodeItem type from @cognite/sdk
Size src/components/Dashboard.tsx — 340 lines, mixes fetch and render logic Extract useDashboardData hook (~120 lines)
DRY src/components/A.tsx , src/components/B.tsx 45, 62 Identical date formatter Extract to src/utils/formatDate.ts
Naming src/hooks/data.ts — File name does not start with use Rename to useData.ts
Dead code src/App.tsx 88 console.log("debug response", data) Remove
If no issues are found in a step, state "No issues found" for that step. Do not skip steps silently.
Done
Summarize the total number of findings by category and list the highest impact items to address first. Any any type and lint error must be treated as blocking — list these separately.