←back to thread

159 points jbredeche | 3 comments | | HN request time: 0.212s | source
Show context
munk-a ◴[] No.45533422[source]
I'm very happy to see the article covering the high labor costs of reviewing code. This may just be my neurodivergent self but I find code in the specific style I write to be much easier to quickly verify since there are habits and customs (very functional leaning) I have around how I approach specific tasks and can easily handwave seeing a certain style of function with the "Let me just double check that I wrote that in the normal manner later" and continue reviewing a top-level piece of logic rather than needing to dive into sub-calls to check for errant side effects or other sneakiness that I need to be on the look out for in peer reviews.

When working with peers I'll pick up on those habits and others and slowly gain a similar level of trust but with agents the styles and approaches have been quite unpredictable and varied - this is probably fair given that different units of logic may be easier to express in different forms but it breaks my review habits in that I keep in mind the developer and can watch for specific faulty patterns I know they tend to fall into while building up trust around their strengths. When reviewing agentic generated code I can trust nothing and have to verify every assumption and that introduces a massive overhead.

My case may sound a bit extreme but in others I've observed similar habits when it comes to reviewing new coworker's code, the first few reviews of a new colleague should always be done with the upmost care to ensure proper usage of any internal tooling, adherence to style, and also as a fallback in case the interview was misleading - overtime you build up trust and can focus more on known complications of the particular task or areas of logic they tend to struggle on while trusting their common code more. When it comes to agentically generated code every review feels like interacting with a brand new coworker and need to be vigilant about sneaky stuff.

replies(1): >>45535937 #
1. never_inline ◴[] No.45535937[source]
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.

replies(2): >>45537343 #>>45542479 #
2. lgas ◴[] No.45537343[source]
> In addition it can assume its writing some throwaway script ...

Do you explicitly tell it that it's writing production code? I find giving it appropriate context prevents or at least improves behaviors like this.

3. munk-a ◴[] No.45542479[source]
> catching every exception and wrapping

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!