Fix Broken PRs: Ace Your Code Review Interview

You're staring at a pull request, five files changed, 200 lines added, and it just… doesn't quite work. The tests are green, sure, but the commit message is "feat: stuff," and a quick local run shows a glaring bug. This isn't just a hypothetical. I’ve been on both sides of code review interviews, and believe me, the "fix a broken PR" scenario is a brutal, yet incredibly effective, filter. It weeds out the folks who can talk a good game from those who actually understand how software gets built and shipped.

Your Mission: Diagnose & Deliver

They aren't looking for perfection, not initially. They’re testing your thought process, your communication, and your ability to wrangle a mess into something shippable. Think of it as a simulated incident response, but for code quality. You’ll usually get a broken, incomplete, or poorly written PR. Your task: identify the problems, explain them, and propose solutions. Sometimes they'll ask you to actually fix it; other times, just the explanation is enough. Always clarify that upfront.

Start by getting your bearings. Open the PR in whatever tool they provide—GitHub, GitLab, or even just a text editor. Don't dive straight into line-by-line comments. First, skim the whole thing. Read the PR description, if there is one. What's the stated goal? Does the code even vaguely attempt to achieve it? Look at the file changes. Are they all related? Does the PR seem too large or too small for the stated feature? This initial sweep gives you context. I usually spend 3-5 minutes on this, just getting a feel for the lay of the land.

Then, you’ll want to get specific. I break down my review into three main categories: functionality, code quality, and process. Each reveals a different facet of your engineering chops.

Functionality: Does It Even Work?

This is the most critical part. If the code doesn't do what it's supposed to, or introduces regressions, nothing else matters. You'll often find a "broken" PR means "broken functionality."

• Understand the Goal: Re-read the PR description, or ask the interviewer for clarification. What problem is this PR trying to solve? Is there a JIRA ticket, a design doc, anything? If it’s missing, that’s your first observation: "The PR description is vague; I'd ask the author for more context on the intended behavior."
• Identify Test Cases: Even if no tests are included, you should formulate them in your head. For a new feature, what are the happy paths? What are the edge cases? What about error conditions? If the PR is supposed to validate an email, you'd think about "valid@domain.com," "invalid," "user@.com," "user@domain," etc.
• Run It (If Possible): If they give you a runnable project, clone it locally. Run the existing tests. If they pass but the feature is broken, that's a huge flag: poor test coverage or incorrect test assertions. Then, try to manually test the feature. Does it blow up? Does it return wrong data? Document exactly what you observe. "When I click the 'Submit' button, the console shows a TypeError: undefined is not a function at line 42 of frontend.js."
• Spot Obvious Bugs: Sometimes it’s a simple off-by-one error, a forgotten null check, or a misconfigured API endpoint. These are easy wins. Point them out clearly.

Don't just say "it's broken." Explain how it's broken and why you think it’s failing. Propose a specific fix, even if it's just pseudocode.

Code Quality: Clarity, Maintainability, & Best Practices

Once you're sure it functions (or have identified its functional flaws), shift your focus. This is where you demonstrate your understanding of good engineering principles.

• Readability & Maintainability:
  • Naming: Are variables, functions, and classes named clearly? processData is worse than calculateOrderTotal. temp is rarely a good name.
  • Complexity: Are functions too long? Do they do too many things? Point out areas for decomposition. "This handleCheckout function is 150 lines; I’d refactor it into smaller, single-responsibility functions like validateCart, processPayment, and updateInventory."
  • Duplication: Are there copy-pasted blocks of code? Suggest helper functions or abstractions.
  • Comments: Are there enough comments? Are there too many comments explaining obvious code? Good code often explains itself, but complex logic or "why" something was done needs a comment.
• Design Patterns & Architecture:
  • Modularity: Is the code tightly coupled? Can parts be reused? Does it adhere to the Single Responsibility Principle?
  • API Design: If it’s an API change, is the new endpoint intuitive? Are the request/response payloads well-structured?
  • Error Handling: Is error handling robust? Are specific errors caught? Are they logged appropriately? Are user-facing errors friendly?
  • Security Concerns: Are there obvious vulnerabilities? SQL injection risks, exposed secrets, unvalidated user input? This is a big one.
• Testing:
  • Coverage: Are there unit tests? Integration tests? If not, that's a major red flag. "This new feature has no test coverage. We need unit tests for calculateDiscount and an integration test for the /checkout endpoint."
  • Quality: Do existing tests actually test the behavior or just the implementation details? Are they readable and fast?
• Specifics: Mention linting issues if you see them (inconsistent formatting, unused imports), or adherence to specific team style guides. If they use TypeScript, are types used effectively? Is there any any usage that could be tightened?

This section isn’t about nitpicking. It's about demonstrating your ability to elevate the codebase, not just patch it.

Process & Collaboration: Beyond the Code

This is often overlooked, but it tells the interviewer a lot about your team player potential. A great engineer isn't just a coder; they're a force multiplier.

• PR Description: Is it clear? Does it explain what was done and why? Does it include screenshots or deployment instructions for new UI? A bad PR description is a communication failure. "The PR description 'feat: stuff' is insufficient. I'd ask the author to detail the problem being solved, the approach taken, and any relevant testing instructions."
• Commit History: Is the commit history clean and logical? Are there atomic commits, or one giant commit that says "WIP"? Suggest squashing or reordering commits for clarity.
• Dependencies: Are new dependencies introduced? Are they justified? Do they add unnecessary bloat?
• Documentation: If this PR introduces a new component or complex logic, does it update relevant documentation (e.g., README, API docs)?
• Trade-offs: Every decision has trade-offs. If you suggest a refactor, acknowledge that it might take extra time. If you propose a new library, mention its maintenance burden. "While adding a new caching layer would improve performance, it also introduces operational complexity. We need to weigh that against the current performance bottlenecks."
• Next Steps: What would you do after this PR is merged? Monitor logs? Plan follow-up tasks?

Remember, you're not just finding problems. You're offering constructive feedback and demonstrating how you’d guide the author to a better solution. Frame your suggestions as questions or collaborative ideas, not demands. "Have you considered X?" or "What if we tried Y?" is better than "You must do Z." This shows empathy and a collaborative spirit, which is huge in a team environment.

The "It Depends" Moment

Sometimes, you’ll encounter a PR that's technically "correct" but incredibly inefficient or overly complex for its stated purpose. This is where your judgment comes in. Do you suggest a complete rewrite that takes a week, or a smaller, incremental improvement that gets it shipped now? There isn't a single right answer. Your response should acknowledge the trade-offs.

"This approach using N-dimensional arrays works, but for our current data scale, a simpler hash map would be far more readable and performant. If we anticipate massive scale in the next 6-12 months, the current approach might be justified, but we'd need to benchmark it. For now, I'd recommend the simpler solution to reduce cognitive load and maintenance." This shows you can think strategically, not just tactically.

Your goal isn't to be the "gotcha" reviewer. It's to be the engineer who makes the team's code, and process, better. They’re looking for someone who can improve code without alienating the author, who can spot both obvious bugs and subtle architectural flaws, and who understands that shipping quality software involves more than just writing lines of code. Practice reviewing real PRs on open source projects or even old code from your own company. That's how you build this muscle.

Ready to Ace Your Next Interview?

Practice with AI-powered mock interviews tailored to your target role and company. Start Practicing for Free | Explore Interview Prep