the tool will adopt a hexagonal architecture

Context¶

The command line tool review-assist (ra)'s current main purpose is to provide an automated review of git change requests on git servers. The automation relies on the LLM inference server the users provide and connect to the tool (see 0001-byo-llm.md ADR). Git servers can be of different kinds (see 0002-git-server-integration.md ADR). The tool doesn't provide a generic code review but has several review lenses which are all invoked by default. All these aspects are managed by the user using a configuration system (see 0003-configuration-management.md).

Whether it's configuration, git servers or inference engine, they are concrete resources that may change over time. The business logic (what the tool does) won't as it is the identity of the tool. As the codebase evolves and gets maintained, both aspects of the tool need to work together to a consistent quality standard and without requiring more effort than necessary for the maintainer (me).

Therefore, it's important to organise the tool in a way that it can be easily extended and maintained.

What about non-functional requirements, like performance and security? Those are orthogonal aspects to the architecture goal, not goals. They should be infused in every cog of the architecture as well as its whole.

Decision¶

Insulate the business logic from infrastructure (git server, configuration, inference engine), and from user interface (console), to provide separation of concerns at the architecture level. Thus, the business logic shouldn't have to be altered if any of the peripheral concerns evolve and vice versa.

Here are the mechanisms we are putting in place to enable that.

(source: hexagonal-arch-diagram.graffle)

Ports and Adapters¶

By using python protocols (interfaces or ports) for adapters to implement, we can isolate the domain logic from the infrastructure changes, since the infrastructure is less stable than the domain logic.

This approach is worthwhile because we will have quite a few adapters to implement (git servers, configuration, inference engine), and we may want to swap implementations in the future (especially for the inference engine as it's a fast-moving technology)

Additionally, the explicitness of the interfaces and the separation of concerns make testing behaviours and integration easier.

In the codebase, all the ports are in the src/reviewassist/ports directory, while most adapters are in src/reviewassist/adapters.

Repository pattern and Service pattern¶

Service and repositories respectively abstract data manipulation and data access to the underlying storage and on underlying compute on behalf of the domain logic. In this architecture, they are ports and are in the code, they are python protocols. Thus, they sit in the src/reviewassist/ports directory.

Concrete classes then implement services and repositories as adapters. However, because they are not strictly infrastructure and are logically important, their implementation will reside in their own directories (src/reviewassist/services and src/reviewassist/repositories).

We will need to have a repository for each kind of data outlet. Lower levels ports describe the interface to infrastructural elements, and their concrete adapter implementations are used in repositories. Services function likewise, but for intelligence interactions.

We will have an infrastructural port for each combination of physical source and access method. For example, * there should be a port for LLM Servers that are accessed via a REST API. * there should be a port for LLM SDK that is embedded in the tool. * there should be a port for centralised git servers that are accessed via REST API. * there should be a port for centralised git servers that are accessed via CLI composition.

There are benefits to having that dual layer of ports: We should be able to easily swap implementations of the infrastructure adhering to the same port protocol, while leaving to the repository the management of the multiple ways data can be accessed, and to the services the management of the intelligence interactions. Thus, the business logic only has to care about accessing the data from the repository (or sending instructions to the intelligence services), irrespective of the where and the how.

The UML diagram below shows that approach in action.

(source: ./repository.d2)

Dependency injection¶

Since the entities of the domain logic have interfaces at the boundary, in the main program we need a way to inject the concrete implementations of those interfaces (dependency injection). If done in the main program, it will make the code very cluttered and unmaintainable.

We need one place to configure (wire) all the concrete implementations of the interfaces (dependency inversion). A sort of registry holding all the concrete implementations, already configured that the main program can use directly.

Trade-offs¶

Alternatives considered¶

Workflow-driven¶

In this approach, what the program does is defined in sequential steps, and each of them is built independently, with a great degree of choice in implementation. The benefit is that I could leverage an appropriate framework for each step (I'm thinking Langchain or LLamaIndex). There are, though, several issues with that approach:

The workflow is hard-coded, and if I want to change it, each step would need to be done as there is not a foundational architectural core to support it. This would have been alleviated by using an embedded workflow manager, but that increases complexity and adds a hard, large dependency and vendor lock-in. The same problem of additional effort on multiple steps happens when adding support for more git servers and inference engines.

The lack of foundational architectural core also increases the risks of design drift between each step, which can generate bugs and make it harder to reason about the code.

OOP Inheritance¶

As an architectural approach, it is not a good choice, it's too rigid and creates a new class of potential issues when multiple inheritance is involved. Furthermore, the main concepts the program needs to handle do not land easily into a hierarchical structure.

That said, tactically, it's a good approach for some use cases, like exception handling, and variations within the domain logic.

Single file¶

Although the very first prototype was a single file, it is very much not the way to go, as we don't want a big ball of mud.

Consequences¶

Such an approach will result in a maintainable and extensible architecture.

Need to be managed¶

Dependencies wiring:¶

We need to implement a DI container for wiring implementations: Maintain a registry of all necessary dependencies in a Dict-like structure. The purpose of the tool is one-shot, so there is no need to develop sophisticated auto-wiring. In a utility module, load configuration, and then all the necessary dependencies based on the CLI arguments.

(source: ./di.d2)

Non-functional requirement:¶

Needs strategies for exceptions and logging so we can make sense of them despite the multiple levels of layering and indirections. * Precise and exhaustive exception class hierarchy * Configurable level of verbosity for diagnostics messages * Collapsible and/or clear labelled tracebacks

Conceptual complexity:¶

• Keep the design documented and tested on multiple levels: acceptance tests (BDD style with features and scenarios), integration.
• Ensure code organisation reflects this approach.

├── features
│   └── steps
├── runtime
│   └── coverage
├── src
│   ├── utils
│   ├── exceptions
│   └── reviewassist
│       ├── ports
│       ├── adapters
│       │     ├── git_servers
│       │     ├── inference
│       │     ├── configuration              
│       │     └── console
│       ├── commands
│       ├── domain
│       ├── services
│       └── repositories
└── tests
    ├── integration
    ├── architecture
    └── unit
        ├── ports
        ├── adapters
        ├── commands
        ├── console
        ├── domain
        ├── repository
        └── utils

• Ensure each python module has docstrings and is not complex. The latter can be verified and enforced with static analysis tools like radon and architecture fitness test frameworks like archunitpython. The former can be verified with plugin flake8-docstrings for flake8 static analysis tool.

Accepted drawbacks¶

• Learning curve