Proposal: Use a Domain-Specific directory structure

[borkweb]

I would like to propose a slight tweak to what we have cooking in #2 with regards to the organization of our classes, enums, interfaces, etc. If I am interpreting the namespaces correctly in that PR, we'd have a structure similar to:

src/
├── AiClient.php
├── MessageBuilder.php
├── PromptBuilder.php
|
├── Enums/
│   └── ...all of the non-provider enums
|
├── ProviderImplementations/
│   ├── Anthropic/
│   |   └── ...
│   ├── Google/
│   |   └── ...
│   └── OpenAI/
│       └── ...
|
├── Providers/
│   ├── Contracts/
│   |   └── ...all of the provider contracts
│   ├── Enums/
│   |   └── ...all of the provider enums
│   ├── Types/
│   |   └── ...all of the provider classes
|   └── Util/
│       └── ...provider utils
|
├── Types/
|   └── ...most of the classes and interfaces
|
└── Util/
    └── ...non-provider utils

Proposed Structure: Domain-specific

A domain-specific structure is pretty good for reducing cognitive load in that it aligns nicely with mental models.

Here's what a domain-specific structure could look like:

src/
├── AiClient.php
|
├── Builders/
│   ├── MessageBuilder.php
│   └── PromptBuilder.php
│
├── Embeddings/
|   |
│   └── DTO/
│       └── Embedding.php
│
├── Files/
|   |
│   ├── Contracts/
│   |   └── File.php
|   |
│   └── DTO/
│       └── InlineFile.php
│       └── LocalFile.php
│       └── RemoteFile.php
|
├── ProviderImplementations/
│   ├── Anthropic/
│   |   └── ...
│   ├── Google/
│   |   └── ...
│   └── OpenAI/
│       └── ...
|
├── Messages/
|   |
│   └── DTO/
│   |   └── Message.php
│   |   └── MessagePart.php
|   |
│   └── Enums/
│       ├── MessageRole.php
│       └── MessagePartType.php
│
├── Operations/
|   |
│   ├── Contracts/
│   |   └── Operation.php
|   |
│   └── DTO/
│       └── EmbeddingOperation.php
│       └── GenerativeAiOperation.php
│
├── Providers/
│   ├── AiProviderRegistry.php
|   |
│   ├── Contracts/
│   |   ├── AiModelMetadataDirectory.php
│   |   ├── Authentication.php
│   |   ├── HttpClient.php
│   |   ├── AiProvider.php
│   |   └── AiProviderAvailability.php
|   |
│   ├── DTO/
│   |   ├── AiProviderMetadata.php
│   |   └── AiProviderModelsMetadata.php
|   |
│   ├── Enums/
│   |   ├── AiProviderType.php
│   |   └── ToolType.php
|   |
│   └── Models/
│       |
│       ├── Contracts/
│       |   ├── AiModel.php
│       |   ├── WithAuthentication.php
│       |   ├── WithEmbeddingOperations.php
│       |   ├── WithGenerativeAiOperations.php
│       |   └── WithHttpClient.php
│       |
│       ├── DTO/
│       |   ├── AiModelConfig.php
│       |   ├── AiModelMetadata.php
│       |   ├── AiModelRequirements.php
│       |   ├── AiRequiredOption.php
│       |   └── AiSupportedOption.php
│       |
│       ├── EmbeddingGeneration/
│       |   └── Contracts/
│       |       ├── AiEmbeddingGenerationModel.php
│       |       └── AiEmbeddingGenerationOperationModel.php
│       |
│       ├── Enums/
│       |   ├── AiCapability.php
│       |   ├── AiModality.php
│       |   ├── AiOption.php
│       |   ├── FinishReason.php
│       |   └── OperationState.php
│       |
│       ├── ImageGeneration/
│       |   └── Contracts/
│       |       ├── AiImageGenerationModel.php
│       |       └── AiImageGenerationOperationModel.php
│       |
│       ├── SpeechGeneration/
│       |   └── Contracts/
│       |       ├── AiSpeechGenerationModel.php
│       |       └── AiSpeechGenerationOperationModel.php
│       |
│       ├── TextGeneration/
│       |   └── Contracts/
│       |       ├── AiTextGenerationModel.php
│       |       └── AiTextGenerationOperationModel.php
│       |
│       └── TextToSpeechConversion/
│       |   └── Contracts/
│       |       ├── AiTextToSpeechConversionModel.php
│       |       └── AiTextToSpeechConversionOperationModel.php
|       |
|       └── Util/
|           └── AiCapabilitiesUtil.php
│
├── Results/
|   |
│   ├── Contracts/
│   |   └── Result.php
|   |
│   └── DTO/
│       ├── Candidate.php
│       ├── EmbeddingResult.php
│       ├── GenerativeAiResult.php
│       └── TokenUsage.php
|    
├── Tools/
|   |
│   └── DTO/
│       ├── FunctionCall.php
│       ├── FunctionDeclaration.php
│       ├── FunctionResponse.php
│       ├── Tool.php
│       └── WebSearch.php
│
└── Util/
    ├── MessageUtil.php
    ├── CandidatesUtil.php
    └── RequirementsUtil.php

Additional suggestion

Perhaps, if we go this route, we could consider removing some of the prefixing and suffixing when it feels duplicative and, instead, rely on the namespaces.

Util example

We could consider dropping the Util part of AiCapabilitiesUtil.

Models example

We could consider dropping the AiModel prefix on AiModelConfig, AiModelMetadata, etc.

[JasonTheAdams]

I believe y'all already discussed it, but for what it's worth my vote would be that Models is a top-level domain and not nested in Providers to reduce domain nesting.

[felixarntz]

I believe y'all already discussed it, but for what it's worth my vote would be that Models is a top-level domain and not nested in Providers to reduce domain nesting.

@borkweb and I discussed this yesterday a bit, and one of the concerns is that there is a lot of overlap between providers and models. I'm wary of unnecessary discussions of "where should this class live" the more separate we keep them. Personally I wouldn't even be opposed of putting their code under one namespace to avoid that confusion - even though generally I'm all for clear separation, in this instance I'm not sure it satisfies the "clear".

If you prefer the separation, I'd say we should at least keep models under providers to make their close relationship more obvious.

[felixarntz]

My other feedback for what's currently outlined in the description:

• The DTO is missing in many places, e.g. under Files, Messages. For Providers\Models it's there, but empty, several of the classes that are currently directly in Providers\Models should be within Providers\Models\DTO.
• For Providers\Models\DTO though, I think some of the classes in Providers\Models would be better to live in the Implementer API side of things. Embedding, Candidate, GenerativeAiResult etc are all data transfer objects that concern both the implementer API and the extender API, and while they're produced by a model / provider, so is Message for example within those responses. It makes more sense to me to keep those data transfer objects somewhere outside of the Providers namespace, as the Providers namespace ideally only contains things that only concern the provider registration and implementation (the extender API).
• I think Messages\Functions should be moved to a top-level Functions. It has a similar role as Files, primarily a set of DTOs to use within Message (or more specifically MessagePart). So I think it should live at the same level.
• Regarding the comment of dropping the AiModel prefix, I would advise not to do that since it clearly separated the names from some similar classes for providers, e.g. consider AiModelMetadata vs AiProviderMetadata. FWIW the same consideration would apply to all the AiProvider classes too.
  • What we could consider instead is dropping simply the Ai prefix from these classes, since you could argue technically everything in here could be prefixed with Ai, so it's somewhat meaningless or even inconsistent.

WDYT @borkweb @JasonTheAdams?

[borkweb]

Regarding the first three bullets, I think it'd be good if you edit the above structure to match what you are thinking so we have something more concrete to lock onto!

What we could consider instead is dropping simply the Ai prefix from these classes, since you could argue technically everything in here could be prefixed with Ai, so it's somewhat meaningless or even inconsistent.

☝️ That's the core of what I'm suggestion - I'd be all for us doing that.

[felixarntz]

@borkweb @JasonTheAdams I updated the issue description with what I'm proposing - I think at least it's very close to it. Please take a look and let me know what you think.

I didn't remove any of the Ai class/interface prefixes, but I think we agree on that, so we can just do that later in the PR.

[felixarntz]

Additional thought: What would you think about giving all contracts (interfaces) names using the Interface suffix? That makes it very easy to spot whether some type hint for example refers to an interface or a concrete DTO or implementation.

Part of me likes not having to add such suffixes, but I think for discoverability and quicker understanding of code they do help. It would also make it easy to consistently use interfaces without getting into some tricky naming situations - for example, if we wanted an interface for a message, that would be MessageInterface, and we wouldn't need to ask ourselves whether Message should be the class or the interface (and if the latter, what would the class be named, which is where things would get weird).

[borkweb]

@felixarntz - great updates!

Additional thought: What would you think about giving all contracts (interfaces) names using the Interface suffix?

I'm of a similar mind as you - I don't love the suffix but it does add clarity. I've personally gone back and forth on this sort of thing in some of my other projects and the absence of the suffix inevitably leads to use ThisThing as ThisThingInterface for clarity and/or to avoid collisions.

My brain says suffix. My heart says no suffix. I think my brain is the right winner here.

TL;DR: My vote is to use the Interface suffix.

[felixarntz]

We should probably capture those decisions in some documentation. Maybe we can create a PR to add NAMING-CONVENTIONS.md? Thinking it could have things like:

• All names must be PSR-12 compatible (typically camel-case for methods, variables, and properties, pascal-case for classes, interfaces, and traits).
• All class, interface, and trait names must match the file names, in accordance with PSR-4.
• All interfaces must be suffixed with Interface.
• All traits must be suffixed with Trait.
• Classes, interfaces, traits, and namespaces, except for the root namespace, must not be prefixed with Ai.

I'd also suggest to suffix all enum classes with Enum. Even more so since we can't use the actual enum type from PHP 8, this will be important.

Anyway, back to the directory structure. If this looks good to you both, then please feel free to update the PR. I can review later.

[JasonTheAdams]

Appreciate the input! I'm, personally, a fan of structuring moderate-to-complex codebases by domain, in one of two patterns:

• Domain > Type
• Domain > Sub-domain > Type

My understanding is that it's perfectly fine for domains to cross-reference. So having a Providers domain reference Domain domain items is perfectly fine. Generally, dependency is in one direct, as in the case the Provider references the Domain. So in determining where something goes in this case, you ask whether it's distinct to Providers (Domains don't reference them), or if Domains or both reference them (put it in the Domain namespace).

Last note with domains is that a domain should be (though not dogmatically) user-facing. That is, if I were explaining this to someone who does not work in this project, what business terms would I use to describe it? In our case we have terms like client, domain, provider, prompt, message, modality, and so forth.

All that said, I won't die on the hill of Models being a root domain. It's a preference, not truly objective.

My brain says suffix. My heart says no suffix. I think my brain is the right winner here.

@borkweb I feel your existential pain. An alternative I'll take here from time to time is some sort of convention instead of a suffix. PHP for example, itself often uses the -able convention (ok, it's still a suffix, but grammatical): Serializable, Traversable. One convention I like is:

• Nouns for core concepts and rules (e.g. File, Model, Provider)
• "With" prefix for mixins (e.g. WithVideoOperations, WithEmbeddings)
• Adjectives for capabilities (e.g. Loggable, Streamable)

That said, sadly, this doesn't negate the -Interface suffix. Honestly, I go back and forth on this more broadly: EventModel, EventRepository, StatusDTO, and so forth.

[borkweb]

That said, sadly, this doesn't negate the -Interface suffix. Honestly, I go back and forth on this more broadly: EventModel, EventRepository, StatusDTO, and so forth.

Should we add DTO as a suffix as well? It does add some extra clarity in a similar way as with Enum and Interface suffixes.

Note: I'm not picky on this one.

I just gave this a try and I think a DTO suffix kind of makes some of the helpful classnames more confusing. Examples: UserMessage, ModelMessage, InlineFile, LocalFile, RemoteFile. Those 5 have the potential to be used directly by implementers a lot, and the DTO suffix is a weird experience...especially if the implementer doesn't know what DTO means.