Code Review

2. Quality, Robustness & Maintainability

Reject the change if it introduces:

• Tangled Flow — Ad-hoc branching or complex conditional trees. → Suggest guard clauses or state machines.
• Scattered Logic — Feature-specific logic placed in shared/common modules. → Suggest moving it to the feature domain.
• Weak Abstraction — Indirection that doesn't simplify or reuse code. → Suggest inlining or reverting to direct calls.
• Redundant State — Duplicate state or cached values that can be derived. → Suggest deriving values dynamically.
• Parameter Sprawl — Function parameter list grows excessively. → Suggest bundling into a config object.
• Stringly-typed Code — Raw strings used instead of constants/unions. → Suggest converting to enums or string union types.
• Loose Typing — Unjustified usage of any, unknown, or casting. → Suggest precise types or type guards.
• Type Boundary Drift — Deserialization/API boundaries are not validated. → Suggest schema validation (e.g., Zod) or type assertions at the boundary.
• Resource Leak — Timers, event listeners, or streams registered without cleanup. → Suggest adding cleanup in teardown hooks (e.g., ngOnDestroy).
• Connection Leak — DB/network handles left open in error paths. → Suggest closing handles in finally blocks.
• Robustness Flaw — Unhandled boundary cases (null, empty, large payloads) or ungraceful crash on network/file failures. → Suggest guard clauses and graceful fallback handling.
• Unsanitized Input — User/external inputs not escaped. → Suggest escaping control characters (e.g. *, _, ` in markdown; <, >, & in HTML).
• Doc Drift — Public API changes lack corresponding JSDoc updates. → Suggest updating documentation.
• Dependency Issues — Missing or unused dependencies in build configs. → Suggest resolving dependencies.

3. Code Reuse & Integration

Flag these opportunities:

• Duplicate Utility — Logic duplicates existing helper functions. → Suggest using the existing helper.
• Hand-rolled Utility — Inline code implements common utility tasks (path handling, environment/flag checks). → Suggest using standard codebase utilities.
• Search Rule — generic-looking code patterns are added without checking. → Actively search for existing helpers using grep_search.

4. Efficiency & Performance

Reject the change if it introduces:

• Waste — Redundant computations, repeated file reads, or duplicate API calls. → Suggest caching or batching.
• Sequential Async — Independent async calls run sequentially. → Suggest combining via Promise.all or Promise.allSettled.
• Hot-Path Bloat — Blocking tasks in startup, request handlers, or render loops. → Suggest async processing or lazy loading.
• TOCTOU Check — Pre-checking resource existence before operating (e.g. checking file existence). → Suggest operating directly and catching errors (e.g. ENOENT).
• Broad Scopes — Reading/loading entire files/datasets when filtering is possible. → Suggest scoped reads or query-level filtering.

5. Unit Test Quality (If tests changed)

Reject the change if it introduces:

• Mock Drift — Mocks that drift from real signatures or use generic any structures. → Suggest type-safe mock frameworks.
• Bespoke Mocks — Manual component/DOM construction in tests. → Suggest reusing testing factories or helpers.
• Weak Assertions — Assertions that are trivial or verify properties unrelated to the behavior under test. → Suggest precise assertions verifying behavioral side-effects.
• Bloated Setup — Repeated boilerplate/mock setups across tests. → Suggest extracting to beforeEach hooks or shared helpers.
• Inconsistent Style — Test structure diverges from surrounding tests. → Suggest aligning with local patterns.
• Missing Coverage — New complex logic or branches added without tests. → Reject change and request new unit tests.