Skip to content

[TRACKING] Battle plan for code quality #399

Description

@rambip

I think it will be a surprise for no one that currently the code quality of this repository is not great.

In the last few weeks, I was increasingly frustrated and held back by code that was really hard to maintain, reason about and refactor.

I think there are ways to greatly improve code quality and I want to discuss them here. I also think that by default it will only go worse as time goes by.

Currently good

There are currently some good ideas put in place to have a decent code quality, like:

  • a lot of unit-tests
  • strict linters locally and in CI
  • 100% typescript
  • code reviews with codex
  • no push on the main branch

That said, these measures have holes that are often exploited by agents.

Currently bad

Agents tend to disable linting rules ( #398 )

Typescript in strict mode still can allow ugly code, with casts everywhere (grep for as in the codebase, it's a pity)

Agents also tend to create weak tests either because they are repetitive, either because they don't test the important invariants, either because they are purposefully too weak to not find the subtle bugs.

A reviewing agent can let ugly code pass if the code was already ugly previously.

When I say ugly code, I think about very specific things:

Solutions

I don't have the full solution yet, but there are things we can try to enforce to improve code quality little by little.

  1. we can use automatic code reviewing tools like CodeRabbit that are more efficient than Codex (or sourcery ).

  2. we can simplify the agents.md file and add a section for style guideline explaining that some parts of the code were written in a bad style and that we should improve it as much as possible and not redoing the same mistakes again. (⚠️agents are quite bad at writing good agents.md file. It would be nice to write it by hand)

  3. Trying to forbid type casts. I'm not sure how to enforce that. Maybe it could be a skill or some automatic tool that could detect the casts on commit. This config could work for that maybe ?

{
  "rules": {
    "@typescript-eslint/consistent-type-assertions": ["error", { "assertionStyle": "never" }],
    "@typescript-eslint/no-explicit-any": "error"
  }
}

But I think if even if the agent get a linting error in this case, it could find a way to fix the error while not refactoring the code in the best way.

  1. having a phase where we don't allow any new features but we focus on code quality only. This is quite common in the software world and we will have to do it one way or another.

  2. having an ARCHITECTURE.md file that explain the entire pipeline in particular the subtle parts so that the agent can reason about the code and take the best decisions

  3. having some prompt/skill that forces the AI to explain the decision he has made and is not sure about, instead of bullshitting. I'm currently experimenting on that kind of thing, but I have a skill you can test already here

  4. Being more careful. You have to suppose that by default the agent is working against you and try to put ugly code and backdoor everywhere. So really push hard so that the agent explain what parts of the code he wrote are probably wrong. In a similar way, saying to the agent, ask as much questions as possible to understand my intent and the code base is very helpful.

Discussion

I would be really interested to get your thoughts on this !

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions