Code Refactor Review

Code Refactor Review

Review code changes the way Sahaj usually asks for review: go deep on reuse, composition, codebase consistency, and anything that reads like slop.

First Pass

1. Inspect the full diff:
   • Use git diff for unstaged changes.
   • Use git diff HEAD if there are staged changes.
   • For a PR URL, inspect the changed files and understand the feature flow before reviewing details.
2. Build the call stack / data flow when useful. Do not review isolated lines without understanding how the feature is wired.
3. Search the codebase before judging new helpers, components, hooks, or patterns. Prefer nearby and sibling patterns over invented abstractions.

Review Lenses

1. Reuse Existing Code

• Look for existing utilities, components, hooks, server actions, route patterns, copy patterns, and styling primitives before accepting newly written code.
• Flag duplicated logic, copied helpers, or custom implementations of things the codebase already has.
• Prefer reusing the existing flow even if it needs a small extension.
• If the new code creates a shared helper, verify it has real reuse and is not just extracted private logic with a vague name.

2. Codebase Consistency

• File placement should match the domain and neighboring features. Be suspicious of random top-level lib dumps.
• Naming should match what the code actually does and follow sibling file/function names.
• Avoid implementation details in names unless they are the actual product/API distinction.
• Use existing result/error/loading patterns. Do not invent bespoke success/failure types when the codebase has a standard one.
• Match existing copy and tone for user-facing text.

3. Composition and Boundaries

• Functions should do one thing at the right level of abstraction.
• Avoid grab-bag modules that mix unrelated concerns like flags, API calls, transformation, UI state, logging, and scheduling.
• Avoid parameter sprawl. If a function needs many knobs, check whether the boundary is wrong.
• Prefer simple composition over chains of callbacks, wrappers, memoized helpers, and prop plumbing.
• When two backing entities are presented as one product concept, package them into one transport/view model across intermediate components. Black-box components should receive one unified prop/callback and should not care about distinctions like remote vs prebuilt; split back into core entities only at roots/adapters where persistence or payload formats require it.
• Keep domain-specific logic close to its domain unless there is proven cross-domain reuse.

4. Slop Detection

Flag and, when asked, remove:

• Comment slop: obvious comments, comments defending awkward code, long comments that should become clearer code, stale context in PR descriptions.
• Helper slop: tiny wrappers that add no meaning, helper files created only to make one function look shorter, unnecessary indirection.
• Type slop: exported one-off types, custom result shapes, annotations where inference is clearer, types that only paper over awkward code.
• Memo/callback slop: useMemo / useCallback added without a measured or structural reason.
• Effect slop: effects that mirror props/state, reset derived state, or handle events after the fact.
• Compatibility cruft: bolted-on behavior that preserves accidental architecture instead of building the coherent end state.
• Diff churn: unrelated renames, formatting, comments, or wrappers that make the PR larger without improving the design.