I have similar OCD behaviors which make reviewing difficult (regardless of AI or coworker code).
specifically:
* Excessive indentation / conditional control flow
* Too verbose error handling, eg: catching every exception and wrapping.
* Absence of typing AND precise documentation, i.e stringly-typed / dictly-typed stuff.
* Hacky stuff. i.e using regex where actual parser from stdlib could've been used.
* Excessive ad-hoc mocking in tests, instead of setting up proper mock objects.
To my irritation, AI does these things.
In addition it can assume its writing some throwaway script and leave comments like:
// In production code handle this error properly
log.printf(......)
I try to follow two things to alleviate this.
* Keep `conventions.md` file in the context which warns about all these things.
* Write and polish the spec in a markdown file before giving it to LLM.
If I can specify the object model (eg: define a class XYZController, which contains the methods which validate and forward to the underlying service), it helps to keep the code the way I want. Otherwise, LLM can be susceptible to "tutorializing" the code.
Our company introduced Q into our review process and it is insane how aggressive Q is about introducing completely inane try catch blocks - often swallowing exceptions in a manner that prevents their proper logging. I can understand wanting to be explicit about exception bubbling and requiring patterns like `try { ... } catch (SpecificException e) { throw e; }` to force awareness of what exceptions may be bubbling up passed the current level but Q often just suggests catch blocks of `{ print e.message; }` which has never been a preferred approach anywhere I have worked.
Q in particular is pretty silly about exceptions in general - it's nice to hear this isn't just us experiencing that!
specifically:
* Excessive indentation / conditional control flow * Too verbose error handling, eg: catching every exception and wrapping. * Absence of typing AND precise documentation, i.e stringly-typed / dictly-typed stuff. * Hacky stuff. i.e using regex where actual parser from stdlib could've been used. * Excessive ad-hoc mocking in tests, instead of setting up proper mock objects.
To my irritation, AI does these things.
In addition it can assume its writing some throwaway script and leave comments like:
I try to follow two things to alleviate this.* Keep `conventions.md` file in the context which warns about all these things. * Write and polish the spec in a markdown file before giving it to LLM.
If I can specify the object model (eg: define a class XYZController, which contains the methods which validate and forward to the underlying service), it helps to keep the code the way I want. Otherwise, LLM can be susceptible to "tutorializing" the code.