As a side note: search for a new job. This one wouldn't get any better.
The goals of the code you are reviewing are:
To ship a feature, which should work according to the requirements.
To reduce the growth of technical debt.
The first goal is reviewed by checking that the unit, integration, system and functional tests are here, that they are relevant, and that they cover all the situations which have to be tested. You also have to check the beliefs the original author may have about the programming language, which could lead to subtle bugs or to the code pretending to do something different from what it actually does.
The second goal is the one your question is focused on. On one hand, new code is not expected to increase technical debt. On the other hand, the scope of the review is the code itself, but in the context of the whole codebase. From there, you, as a reviewer, can expect two approaches from the original author:
The outside code is not my fault. I just implement the feature and don't care about the whole codebase.
In this perspective, the code will copy the flaws of the codebase, and so inevitably increase the technical debt: more bad code is always worse.
While this is a valid short-term approach, in long term, it would result in increasing delays and low productivity, and eventually lead to development process being so expensive and risky, that the product would stop evolving.
Writing new code is an opportunity to refactor legacy one.
In this perspective, the effect of the flaws of the legacy code on the new one could be limited. Moreover, the technical debt could be reduced, or at least not increased proportionally to the code growth.
While this is a valid long-term approach, it has its short-term risks. The major one is that, short-term, it would sometimes take more time to ship the specific feature. Another important aspect is that if the legacy code is untested, refactoring it presents a huge risk of introducing regressions.
Depending on the perspective you want to encourage, you may be inclined to advise the reviewees to refactor more or not. In all cases, don't expect flawless, clean piece of code with nice architecture and design inside a crappy codebase. What you shouldn't encourage is the behavior where a knowledgeable developer who has to work on a crappy codebase tries to do his part well. Instead of making things simpler, it only makes them more complicated then before. Now, instead of uniform bad code, you have a part with design patterns, another part with clean, clear code, another part which was extensively refactored over time, and no unity whatsoever.
Imagine, for instance, that you are discovering a legacy codebase of a medium-size website. You're surprised by the lack of any usual structure and the fact that the logging, when it's done, is done by appending stuff to a text file by hand, instead of using a logging framework. You decide for the new feature to use MVC and a logging framework.
Your colleague is implementing another feature and is very surprised by the lack of an ORM where one would make perfect size. So he starts using an ORM.
Neither you, nor your colleague is able to go through hundreds of thousands of lines of code to make the use of MVC, or a logging framework, or an ORM everywhere. Actually, it would require months of work: imagine introducing MVC; how long would it take? Or what about an ORM in situations where SQL queries were chaotically generated through concatenation (with occasional places for SQL Injection) inside code no one could understand?
You think you did a great job, but now, a new developer who joins the project have to face much more complexity than before:
The old way of treating the requests,
The MVC way,
The old logging mechanism,
The logging framework,
The direct access to the database with SQL queries built on the fly,
The ORM.
On one project I was working, there were four (!) logging frameworks used side by side (plus manual logging). The reason is that every time someone wanted to log stuff there was no common approach to do it, so instead of learning a new framework (which in all cases was used only in 5% of the codebase), one would simply add another one he already knows. Imagine the mess.
A better approach would be to refactor the codebase one step at a time. Taking once again the example of logging, refactoring would consist of the following small steps:
Find all the places where legacy logging is done (i.e. when the log file is accessed directly) and ensure they all call the same methods.
Move this code to a dedicated library, if applicable. I don't want logging storage logic in my shopping cart class.
Modify, if needed, the interface of the logging methods. For instance, we can add a level indicating whether the message is informal, or is a warning or an error.
Use the newly refactored methods in the new feature.
Migrate to the logging framework: the only affected code is the code within the dedicated library.
