Maintainability code review rules

Store repeated string literals in constants or variables to improve maintainability and reduce redundancy.

Detect methods that have an empty body. If a method is empty, ensure that a comment is present explaining why it remains unimplemented.

Detect modifications to built-in prototypes. Modifying built-in objects can lead to compatibility issues and unexpected behavior. Recommend creating helper functions instead.

Before persisting ePHI, encrypt using a data key protected by a Key Management Service (KMS). Use authenticated encryption (AES-256-GCM or equivalent), rotate keys, and store the key id and algorithm with the record.

All static JS/CSS/font/image files MUST use content-hashed filenames (e.g., app.9c1a7b.js) and be served with "Cache-Control: public, max-age=31536000, immutable". HTML and other non-fingerprinted documents MUST be served with "Cache-Control: no-cache" (or equivalent) to enable conditional revalidation.

Ensure close/stop/dispose is called for resources (clients, channels, scopes) using use{} or try/finally; cancel coroutines on shutdown.

Check that finalizers are not left empty. If cleanup is needed, ensure proper resource disposal is implemented instead.

Do not combine mechanical changes (formatting, renames) with behavioral changes. If unavoidable, separate commits and call out which commits are mechanical.

Classes with only private constructors cannot be instantiated outside their own scope, potentially making them dead code.

Detect the use of the 'any' type in TypeScript. Using 'any' disables type checking and can lead to runtime errors. Recommend using specific types or generics instead.

Detect cases of unsafe type assertions. These do not perform runtime checks and can lead to unexpected runtime errors. Recommend using proper type guards instead.

Ensure that `File.deleteOnExit()` is not used, as it can cause memory and resource leaks.

Ensure that the `finalize()` method is not used. Use try-with-resources or explicit cleanup methods instead.

Check for global variable usage (e.g., variables prefixed with `$`). Suggest using instance variables, constants, or dependency injection instead.

Identify global variables that are mutable (e.g., lists, dictionaries). Global mutable state can cause unpredictable behavior. Recommend using local variables or encapsulating state in a class.

Tag records by data_class (PII, telemetry, auth) and enforce retention (e.g., PII ≤ 365 days unless legal hold). Provide scheduled deletion jobs and audit log entries.

Check if modules export mutable variables. Mutable exports can be modified by other modules, leading to unpredictable behavior. Recommend exporting immutable values instead.

Check for empty code blocks (such as `if` or `loop` structures) that do not contain any statements. If an empty block is present, ensure it includes a comment explaining its purpose.

Detect occurrences of the 'with' statement. This construct is banned in strict mode due to unpredictable behavior and makes debugging harder.

Every package must have a package comment, and each exported identifier must have a comment starting with the identifier name and a full-sentence summary.

All public modules, classes, and functions must include a docstring immediately after the definition, using PEP 257 conventions (one-line summary + optional sections like Args, Returns, Raises).

Identify functions that raise exceptions but do not document them in docstrings. Missing exception documentation makes it difficult to understand failure cases. Suggest adding an `:raises` section in the docstring.

When a function is unsafe, may panic, or returns Result, include sections # Safety, # Panics, and/or error conditions in the docs.

On every create/read/update/delete of CHD or tokens, write a structured audit event (who, what, when, result) without full PAN, including only pan_last4. Persist to an append-only/immutable sink. (PCI DSS 4.0 Req. 10)

Root package.json must define the packageManager field (e.g., "pnpm@9.x") and the repo must not contain other lockfiles or tool config (e.g., yarn.lock, package-lock.json) conflicting with that choice.

Do not violate the project's architecture boundaries. Examples of violations: Controllers/Views directly querying the database; UI importing infrastructure modules; domain modules importing unrelated domains. If the repo defines boundaries (lint rules, Nx tags, dependency-cruiser, ARCHITECTURE.md), enforce them. When in doubt, route all I/O through the intended service/repository layer and keep domain logic isolated.

Ensure that `StringBuilder` instances are converted to strings using `ToString()` before use to prevent memory wastage.

Copy-pasting code can lead to errors where getters or setters access the wrong fields. Always verify that properties interact with the expected fields.

All public classes, interfaces, and methods must include Javadoc with a summary sentence, @param for each parameter, @return when not void, and @throws where applicable.

All exported JS/TS functions, classes, and public members must include JSDoc with a summary and @param/@returns; include @throws when applicable.

Docstrings must match parameter names, optional defaults, return type, and raised exceptions; update the docstring in the same PR as any signature change.

Ensure that variables that never change are marked as `const` to prevent accidental modification and enhance code clarity.

Ensure the PR title/description honestly reflects what the diff actually changes. Flag out-of-scope changes (behavioral changes hidden as refactors, unrelated dependency bumps, config/security changes) and require either: - splitting into separate PRs, or - an explicit 'Additional changes' section with rationale and risk/rollout notes.

Identify function parameters that are declared but never used within the function body. Remove them to improve clarity and maintainability.

If a PR modifies a workspace with "private": false in its package.json, enforce a .changeset/*.md entry describing semver impact for each affected package.

Do not rewrite accepted ADRs. When reversing or replacing a decision, create a new ADR with Status: accepted and add 'Supersedes: NNNN' while marking the old ADR as 'Status: superseded by NNNN'.

All Terraform/Kubernetes resources must include standard tags/labels: owner, env, cost-center (and application if available).

Every ADR must contain the sections: Context, Decision, Consequences, Status. Allowed Status values: proposed, accepted, deprecated, superseded.

Configure a remote backend with state locking (e.g., S3 + DynamoDB table or AzureRM blob with leases). Do not use local backend for shared environments.

For changes affecting public APIs or modules, include 'Versioning & Deprecation' with semantic version impact and deprecation timeline; link to CHANGELOG and deprecation notices.

Ensure that constants are declared with the `final` keyword to prevent modification.

Ensure that immutable instance fields are marked as `readonly`, and compile-time constants are marked as `const`.

Store decision records under docs/adr/ with zero-padded incremental numbering and a kebab-case title: docs/adr/NNNN-title.md (e.g., docs/adr/0001-choose-postgresql.md).

Ensure that Java records are used for immutable data structures instead of manually implementing immutable classes.

Make utility classes non-instantiable by providing a private constructor or marking them as `abstract`.

Provide a decorator/context manager that opens/closes DB sessions and handles commit/rollback automatically.

Ensure that all classes, fields, and methods are documented with JSDoc. JSDoc improves code clarity, provides documentation for developers, and helps with IDE autocompletion.

Add a CI job that validates docs/adr/ files: filename pattern, required sections, valid Status, presence of PR/issue links, and optional schema (YAML front matter).

Do not put complex logic or long-running tasks in package `init()` functions. Keep `init()` minimal (if you use it at all), such as simple registration of types or default variable initialization. Perform real setup in explicit functions (like in `main` or a setup function called by main).

Ensure that console statements are removed in production code. Console logs can expose sensitive data and impact performance.

Detect the use of default exports. Named exports provide better clarity, maintainability, and prevent issues when refactoring.

Ensure that classes with only static members are converted to top-level functions or extensions. Avoid unnecessary class instantiation.

Flag usages marked as @deprecated (via JSDoc or types) and propose the documented alternative when available.

Treat function inputs as immutable; copy before mutation (e.g., dict.copy(), list(x)).

Do not use global (package-level) variables for mutable state whenever possible. Instead, encapsulate state in structs or pass it as parameters. If you must use a global variable (for configuration or caching), protect it with mutexes if concurrent access is possible and document its usage.

Ensure that arrays do not contain non-numeric properties. This practice can lead to unexpected behavior. Use Maps or Objects instead.

Do not use global variables to share data across your program. Prefer passing values as function parameters or using configuration objects. Limiting variable scope makes code more predictable and less prone to side effects.

Methods returning values that are never used introduce unnecessary complexity. Remove such return values to improve clarity.

Check for the use of getters and setters. These can introduce unexpected side effects and have limited compiler support. Recommend using regular methods unless required by a framework.

Use Directory.Packages.props at the repo root to declare PackageVersion entries. Workspace .csproj files must reference packages without embedding Version attributes.

Route all analytics events through a single service/helper instead of calling the SDK directly from widgets and blocs.

Define configuration values in one place (env/config file/const class) and reference them; avoid scattering magic strings.

Generate cache/Redis keys through a single utility with clear namespaces and versioning.

In the root Cargo.toml, define [workspace] members and manage shared versions under [workspace.dependencies]. Member Cargo.toml files must not pin versions for crates declared centrally.

Define shared string literals (route names, keys, claim types) as constants/enums in a single place.

When removing or decommissioning a feature, ensure the associated feature flag is also removed from: - application code paths, - config/registry files (e.g., flags.ts, config.json), and - analytics/feature flag tools (if applicable). If a PostHog (or similar) MCP is available, verify the flag is disabled/removed and not referenced elsewhere. Otherwise, require a repo-wide search evidence in the PR.

Avoid maintaining multiple equivalent validators for the same shape; centralize checks in a single function/schema.

Replace repeated set/get config calls with data-driven loops or loaders from a single schema/source.

Run a scheduled job that deletes telemetry after 30 days and anonymizes PII after the retention window; log deletions with data_class and count.

Replace magic strings with constants or enums sealed classes; keep them in a single source of truth.

Create theme-like constants (radii, gaps, durations) and reuse across widgets.

If the PR introduces or expands usage of deprecated libraries/patterns the team is migrating away from (e.g., Moment.js, React Class Components), block and suggest the modern replacement. If the repo includes a migration guide (e.g., `docs/migration.md`, `CONTRIBUTING.md`, `docs/adr/*`), follow it and propose the equivalent snippet. If no guide exists, use an available web/search MCP to propose a safe, modern alternative and ask for confirmation of project standards.

Avoid functions or extensions deprecated in recent PHP versions (e.g., mysql_* or preg_replace without proper delimiters). Use modern supported alternatives like PDO/MySQLi for DB or PCRE functions (preg_match, preg_replace with correct syntax).

When a TS package depends on another workspace package, tsconfig.json must set "composite": true and include a "references" entry to the dependency; root must aggregate paths via tsconfig.base.json if used.

Move magic values (strings, thresholds, paths) to module-level constants with descriptive names.

When the same set of attribute/field names is used in multiple places (strong params, serializers, queries), extract it to a constant and reuse.

Move repeated sequences of operations into named helpers or utilities and reuse them.

Parameterize tool/runtime versions via ARGs (and CI build args) instead of hardcoding them across multiple Dockerfiles.

Replace raw numeric/string literals used for dimensions, durations, and thresholds with named constants.

Define SQL strings as static final constants or in a repository, not inline across methods.

Centralize queue names and reuse across workers and tests.

The first sentence should be a standalone summary used by godoc. For functions returning error, state error conditions succinctly.

Keep public types in files with matching names (one top-level type per file) to improve navigation.

Freeze constant objects (arrays/hashes/strings) to avoid accidental mutation at runtime.

List all included builds/modules in settings.gradle and manage versions via gradle/libs.versions.toml (version catalogs). Forbid hardcoded versions and repo declarations in module build.gradle files.

Use template helpers to set metadata.name and labels (app.kubernetes.io/) consistently across templates; prohibit hardcoded mismatches.

Changes that create new ePHI stores must also include retention configuration and a scheduled purge job aligned to policy (e.g., 6 years). Document tables/collections covered and add tests for TTL behavior.

When a PR implements or changes a decision, include 'ADR: docs/adr/NNNN-title.md' in the PR description and reference it in the merge commit body.

Reference at least one tracking ID (e.g., JIRA/GitHub issue) in the PR description using a consistent format (e.g., 'Refs: ABC-123').

Keep docs/adr/README.md updated with a table of ADRs (number, title, status, date). Update the index in the same PR that adds/changes an ADR.

Expose constant dicts as read-only via MappingProxyType (or frozen dataclasses) to prevent runtime mutation.

Check if properties that are never reassigned are marked as readonly. This improves code clarity and prevents unintended reassignment.

Store long static templates/prompts in files and load them at runtime; avoid embedding huge multi-line strings in code.

Check if type aliases are used instead of interfaces. Interfaces provide better extensibility and readability when defining object shapes.

Avoid re-creating existing UI components or duplicating styles (CSS/Tailwind) when the company design system already provides an official component. If the PR touches UI and a design system entry point is referenced (e.g., `ui/index.ts`, `design-system.md`), use it as the source of truth and suggest the equivalent official component.

Check for functions with unused parameters and ensure they are titled with `_` or `__` to indicate they are intentionally unused.

Ensure that isMounted is not used in React components. This method is deprecated and can cause unreliable state updates. Suggest using cleanup functions or useRef instead.

Centralize repeated validation rules into helpers or small packages. Reuse them across handlers, services, and jobs.

Do not include commented-out code, unused files, or unrelated snapshots. Keep diffs minimal and relevant to the change.

Delete unreachable branches and unused methods; rely on VCS for history.

Delete Console.WriteLine, Debug.WriteLine, and temporary diagnostics before merging; rely on structured logging.

Do not define the same method multiple times; consolidate behavior in one place.

Avoid no-op or duplicate calls (e.g., flatten.flatten, compact followed by compact!).

Eliminate debug helpers (var_dump, die, print_r) and commented-out blocks before merging.

Detect lines with trailing whitespace. Trailing spaces add unnecessary noise to version control diffs. Recommend removing them to keep the codebase clean.

Delete dead code and after-return/throw statements that never execute.

Eliminate assignments to variables that are never used or overwritten immediately to improve code efficiency and readability.

Delete dead configuration fields and env flags; keep only variables that change behavior.

Delete imports that are not referenced by the compilation unit.

Delete imports and locals that are not referenced; prefer tooling (ruff/flake8/mypy) to enforce.

Delete dead/commented code and unused params; rely on version control for history.

Delete unused locals or prefix with underscore when required by interface.

Derive paths/URLs from configuration or environment variables, not string literals in code.

Give semantic names to magic numbers/strings and freeze where appropriate.

Extract numeric literals with meaning into named constants or readonly fields.

Define scalaVersion and library versions in version.sbt or ThisBuild in build.sbt; modules must not override scalaVersion or pin conflicting dependency versions locally.

On Sev1/Sev2 incidents, capture timeline, root cause, remediation, and owner in a postmortem within 5 business days; link incident_id in all related commits and changes.

Ensure that string refs are not used in React. String refs are deprecated, make components harder to maintain, and reduce composability. Recommend using function-based refs instead.

In pnpm-workspace.yaml, explicitly list workspace globs (e.g., "apps/", "packages/") and exclude build outputs (e.g., "/dist", "/build"). Forbid catch-all patterns like "*" or "".

If a PR adds new TODO/FIXME comments, require a tracking ticket. Preferred formats: `TODO(ABC-123): ...` or `FIXME(ABC-123): ...`. If a ticketing MCP is available, suggest (or create) the ticket and link it in the PR. Avoid leaving untracked TODOs in mainline code.

When introducing or deprecating public APIs, include @since for introductions and @deprecated with migration notes and links for removals.

Use <inheritdoc/> to inherit docs for overrides/implementations, but explicitly add <exception> and behavior differences when applicable.

Detect relative imports (e.g., `from .module import x`). Relative imports can break if the project structure changes. Recommend using absolute imports (`from package.module import x`) for better clarity and maintainability.

When a value comes from a finite set, model it as an enum constant instead of string/int literals.

Replace string literals that represent finite sets with enums (optionally flagged) or constants.

Declare SEO metadata via export const metadata or generateMetadata; avoid manual <Head> in App Router.

Replace numeric/string literals with named constants to document meaning.

When emitting custom events with data using `$emit`, pass an object literal as the payload instead of a raw primitive value, especially if there's a chance more data might be needed later.

Do not hardcode UI strings; use I18n keys with proper scopes.

Check if function signatures include type annotations. Type hints improve readability and help catch type-related errors early. Recommend adding type annotations where missing.

Ensure that functions in public APIs include type annotations. Type annotations improve code clarity and help catch type-related errors early. Recommend adding them where missing.

Utility classes are not meant to be instantiated. Mark them as static or provide a private constructor to prevent unintended instantiation.

Accept only short-lived tokens from the gateway (JWT/JWE or opaque) and validate signature/expiry; do not persist tokens beyond business need. Never attempt to reconstruct PAN from tokens. (PCI DSS data minimization)