You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Feat!]: Use records for immutable objects in the spec API #507
Is your feature request related to a problem? Please describe.
The domain objects in spec are immutable but use either class or record for their definitions.
That makes the api quite inconsistent (eg task.getTaskId() vs artifact.artifactId()).
Use records for the domain object would be more consistent and helps with #506.
This would be a breaking change in the API though...
Describe the solution you'd like
Use records for the domain API so that the API naming conventions and usage would be more consistent.
It should at least be done for the "payloads" (Task, Message, update events, etc.) and the agent card (and its attributes).
I can't remember exactly, but I think originally they were all records, but then we had to change some of them to classes. It could possibly be due to inheritance, but I can't remember. In any case it seems with records that you run into issues where they don't work any longer, so they might be better for very simple stuff.
+1 on the consistency, and that has been slightly bothering me for some time. One option could be to rename the class getters to look like record getters. But at the same time I think the MapStruct stuff relies on classes being beans, so that will probably break.
Maybe instead we should make all the records into classes with getXXX() methods? We could deprecate the existing artifactId() type methods to make the migration less painful.
Actually, looking at the inheritance of some (not all of the classes), it seems we are extending (sealed) abstract base classes in a lot of cases. However, it seems record can implement interfaces. so maybe the abstract classes can be converted to that.
I think the reason for the abstract base classes could have been the Jackson serialization.
So perhaps records can work anyway. In any case, I agree we should choose one of the two options
Activity
kabir commentedon Dec 4, 2025
I can't remember exactly, but I think originally they were all records, but then we had to change some of them to classes. It could possibly be due to inheritance, but I can't remember. In any case it seems with records that you run into issues where they don't work any longer, so they might be better for very simple stuff.
+1 on the consistency, and that has been slightly bothering me for some time. One option could be to rename the class getters to look like record getters. But at the same time I think the MapStruct stuff relies on classes being beans, so that will probably break.
Maybe instead we should make all the records into classes with getXXX() methods? We could deprecate the existing artifactId() type methods to make the migration less painful.
kabir commentedon Dec 4, 2025
Actually, looking at the inheritance of some (not all of the classes), it seems we are extending (sealed) abstract base classes in a lot of cases. However, it seems record can implement interfaces. so maybe the abstract classes can be converted to that.
I think the reason for the abstract base classes could have been the Jackson serialization.
So perhaps records can work anyway. In any case, I agree we should choose one of the two options
jmesnil commentedon Dec 4, 2025
yes, I'm not sure how sealing an interface affects records that implements them.
I'll try to refactor a few different types to see if I'm facing any issues.
feat!: Unify spec domain classes as records (#548)
feat!: Unify spec domain classes as records (a2aproject#548)