This repository was archived by the owner on Jul 16, 2025. It is now read-only.
feat: use intersection type AbstractUid&TimeBasedUidInterface for flexible message IDs
#384
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OskarStark
changed the title
getDateTimegetDateTime
valtzu
reviewed
Jul 10, 2025
Member
|
Nice one, good to merge after PHPStan fixes :) 👍 |
Contributor
Author
|
Hmm, I don't really know what to do: Require |
Member
|
what about |
OskarStark
changed the title
getDateTime
OskarStark
changed the title
Uuid&TimeBasedUidInterface for message IDs
OskarStark
changed the title
Uuid&TimeBasedUidInterface for message IDs
OskarStark
changed the title
AbstractUid&TimeBasedUidInterface for flexible message IDs
Contributor
Author
|
cc @llupa |
Contributor
Author
|
Good to go from my side @DZunke @chr-hertel |
…ible message IDs This PR updates the message ID type from TimeBasedUidInterface to AbstractUid&TimeBasedUidInterface using PHP's intersection types. Problem: - The original implementation returned TimeBasedUidInterface from getId(), but tests were calling toRfc4122() which is not part of that interface - This caused PHPStan errors Solution: - Using the intersection type AbstractUid&TimeBasedUidInterface ensures that both toRfc4122() (from AbstractUid) and getDateTime() (from TimeBasedUidInterface) are available - This allows implementations to use any time-based UID type (UUID v1/v6/v7 or ULID) Changes: - Updated MessageInterface::getId() return type - Updated all message implementations (AssistantMessage, SystemMessage, UserMessage, ToolCallMessage) - Added tests to verify interface implementations - Updated PHPStan configuration Benefits: - Flexibility: Users can now implement messages with either UUID or ULID - Type Safety: The intersection type guarantees all necessary methods are available - Future Proof: Any new Symfony UID types that extend AbstractUid and implement TimeBasedUidInterface will work - Backwards Compatible: All existing UUID implementations already satisfy the new type constraint
chr-hertel
approved these changes
Jul 14, 2025
Member
chr-hertel
left a comment
chr-hertel
left a comment
There was a problem hiding this comment.
Let's get this merged and than cherry picked :)
Contributor
Author
|
fyi @llupa |
OskarStark
added a commit
to symfony/ai
that referenced
this pull request
Jul 15, 2025
…idInterface` for flexible message IDs (OskarStark) This PR was merged into the main branch. Discussion ---------- [Platform] Use intersection type `AbstractUid&TimeBasedUidInterface` for flexible message IDs | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Docs? | no | Issues | | License | MIT Cherry picking php-llm/llm-chain#384 Commits ------- 115a9e1 feat: use intersection type `AbstractUid&TimeBasedUidInterface` for flexible message IDs (#384)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR updates the message ID type from
TimeBasedUidInterfacetoAbstractUid&TimeBasedUidInterfaceusing PHP's intersection types. This change provides more flexibility while maintaining type safety.Solution
Using the intersection type
AbstractUid&TimeBasedUidInterfaceensures that:toRfc4122()(from AbstractUid) andgetDateTime()(from TimeBasedUidInterface) are availableChanges
MessageInterface::getId()return typeBenefits
Breaking Changes
None - this is backwards compatible since all existing UUID implementations already satisfy the new type constraint.
Testing
All existing tests pass. The implementation has been verified to work with both UUID and ULID.