Skip to content

feat: open contract for more flexible normalizer configuration #372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 5, 2025

Conversation

DZunke
Copy link
Contributor

@DZunke DZunke commented Jul 1, 2025

Could fix part of #371 - the question for flexibility.

Why? Messages are given on platform calls and are given with the MessageInterface. So it is very flexible what could be given on call. Normalizers are only configured once, internally. So one side is highly flexible, the other side not.

Giving the contract an interface allows creating own contracts with whatever backed technology one wants. So more flexible. Additionally it allows to add a contract that has an amount of normalizers one wants. For making it easier i thought about having model contract sets. I did it for anthropic, google and the platform itself.

So a custom contract could look like

use PhpLlm\LlmChain\Platform\Bridge\OpenAI\PlatformFactory;
use PhpLlm\LlmChain\Platform\Contract;

$platform = PlatformFactory::create(
    'my-api-key',
    contract: Contract::create(new MyOwnMessageBagNormalizer(), new MyOwnMessageThingyNormalizer()),
);

or, totally flexible:

use PhpLlm\LlmChain\Platform\Bridge\OpenAI\PlatformFactory;

$platform = PlatformFactory::create(
    'my-api-key',
    contract: new MyVeryOwnContractUtilizingJsonStreamer(),
);

Maybe this could be a way for more flexibility with the combination of non configurable normalizer and "unknown" message input?

@OskarStark OskarStark changed the title feat: contract interface for more flexible normalizer configuration feat: add ContractInterface for more flexible normalizer configuration Jul 1, 2025
use PhpLlm\LlmChain\Platform\Bridge\Anthropic\Contract\MessageBagNormalizer;
use PhpLlm\LlmChain\Platform\Bridge\Anthropic\Contract\ToolCallMessageNormalizer;
use PhpLlm\LlmChain\Platform\Bridge\Anthropic\Contract\ToolNormalizer;
use PhpLlm\LlmChain\Platform\Bridge\Anthropic\Contract\AnthropicSet as AnthropicContractSet;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need the alias?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. Removed it.

/**
* @author Denis Zunke <[email protected]>
*/
final readonly class AnthropicSet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
final readonly class AnthropicSet
final readonly class AnthropicDefaultContract
Suggested change
final readonly class AnthropicSet
final readonly class AnthropicContract

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah it is not a contract yet, hmm 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I have seen that there are Contract Builds that combine mixed model contract normalizers so my thought here was to give them as a "set" like rector is doing it for some tools. So you could combine the set. Maybe this is too much? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

@DZunke DZunke force-pushed the extendable-contract branch from dd80885 to c5a5d9b Compare July 3, 2025 10:09
@OskarStark OskarStark added the enhancement New feature or request label Jul 3, 2025
@chr-hertel
Copy link
Member

Looks great at the first glance - need to give it a proper read later - thanks already @DZunke!

Comment on lines 12 to 28
final readonly class AnthropicSet
{
/** @return array<NormalizerInterface> */
public static function get(): array
{
return [
new AssistantMessageNormalizer(),
new DocumentNormalizer(),
new DocumentUrlNormalizer(),
new ImageNormalizer(),
new ImageUrlNormalizer(),
new MessageBagNormalizer(),
new ToolCallMessageNormalizer(),
new ToolNormalizer(),
];
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about just

final class AnthropicContract extends Contract
{
    public static function create(): self
    {
        return parent::create([
            new AssistantMessageNormalizer(),
            new DocumentNormalizer(),
            new DocumentUrlNormalizer(),
            new ImageNormalizer(),
            new ImageUrlNormalizer(),
            new MessageBagNormalizer(),
            new ToolCallMessageNormalizer(),
            new ToolNormalizer(),
        ]);
    }
}

would need some changes with the parent, but would ease the handling while instantiating the Platform:

$contract ?? AnthropicContract::create()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This feels like a good improvement alternative to the rector inspired sets 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have opened up the Contract and removed the ContractInterface. For this i made the non overwritable methods final instead of the class and only create can be extended. Feels also very native usable to me. What do you think about the changes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like 😍

@DZunke DZunke force-pushed the extendable-contract branch from c5a5d9b to b55d08b Compare July 5, 2025 08:29
@DZunke DZunke changed the title feat: add ContractInterface for more flexible normalizer configuration feat: open Contract for more flexible normalizer configuration Jul 5, 2025
@DZunke DZunke changed the title feat: open Contract for more flexible normalizer configuration feat: open contract for more flexible normalizer configuration Jul 5, 2025
@DZunke DZunke force-pushed the extendable-contract branch from b55d08b to df30ccb Compare July 5, 2025 08:33
Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @DZunke - code looks good & functional tests are good as well :)

@chr-hertel chr-hertel merged commit c9e2ad9 into php-llm:main Jul 5, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants