Add aff for object type SIA6 - #838
Conversation
Control PanelHi, I'm an AI-powered Review Bot that helps you with summarizing and reviewing pull requests.
|
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds solid AFF support for the SIA6 (IAM App) object type with a well-structured ABAP interface, JSON schema, example, and README. The main issues to address are: the missing ty_general_information grouping structure (a structural requirement for object types with metadata beyond the header), the non-standard example file name, and the use of STANDARD TABLE where SORTED TABLE WITH UNIQUE KEY should be preferred for identified list entries.
PR Bot Information
Version: 1.29.18
- Review Focus Files:
- File Content Strategy: Full file content
- Agent Instructions:
- Event Trigger:
pull_request.ready_for_review - LLM:
anthropic--claude-4.6-sonnet - Correlation ID:
33532470-9499-11f1-9078-388357d22c4e
There was a problem hiding this comment.
The PR is well-structured overall — ty_general_information, sorted tables with unique keys, and proper ABAP Doc annotations are all present. Two issues were flagged: the general_information field in ty_main is missing $required, which would allow the entire block (including the mandatory type field) to be omitted from valid JSON; and the ty_services table uses a composite key id type rather than id alone, which conflicts with id being described as the service identifier. Please also address the previously raised comments from other reviewers regarding the example file naming convention.
PR Bot Information
Version: 1.29.18
- Event Trigger:
pull_request.ready_for_review - Review Focus Files:
- Agent Instructions:
- Correlation ID:
5a3781c0-94b3-11f1-82fe-1ee30bb6767a - File Content Strategy: Full file content
- LLM:
anthropic--claude-4.6-sonnet
Markus1812
left a comment
Markus1812
left a comment
There was a problem hiding this comment.
Hi, thanks for contributing your AFF! Might look like a lot of comments, but there are essentially two different kinds of questions/comments:
- If you have predefined values for a field, you can use an enum. LLMs can take massive advantage of this, as enums are contained in the JSON schema that is exposed via an MCP tool. Without that, they can only guess the value of some fields and have to rely on the syntax check or other validations to give them more input. Would be great, if you could introduce some enums, where possible. I've linked the documentation for that in the first comment related to that.
- The other comments are mostly about the naming. As AFFs have the two main usages of the VS Code editor (that shows camel case attributes in the JSON) and the server driven UI (which shows titles), we try to keep component names and titles in sync. I know, server-driven UI is currently not on the roadmap, but we want to be prepared :). I've made some naming suggestions. Please consider them as suggestions, other suggestions are also welcome.
Best regards, Markus
| "description": "Example service", | ||
| "type": "odataV4", | ||
| "serviceId": "30C9E656A21BF6F30B3DA55ABE8BF8", | ||
| "uiadIdSource": "", |
There was a problem hiding this comment.
all these fields that are empty in the example should be removed, as the default values are not serialised. At least for strings, the default is empty, so the field does not need to be in the json, as it is implied
| BEGIN OF ty_service, | ||
|
|
||
| "! <p class="shorttext">Service Name</p> | ||
| "! Name of the service |
There was a problem hiding this comment.
| "! Name of the service | |
| "! Service name |
When the description is not providing any extra information, we recommend using the exact same expression as in the title, but in sentence case. So Service Name -> Service name
|
Hi, thanks for the updates. One thing to keep in mind: When working with enums in AFF, keep in mind that adding a new value down the road is actually an incompatible change. The only way around this is if you've already defined a why: imagine a newer system writes a JSON file using your new enum value, but an older system tries to read it. Since the older system has no idea what this new value means and has no default fallback to drop back to, it will choke on the file and fail to deserialize it entirely. (Note: As long as the AFF is only used in VS Code with the AffAdapter, incompatible changes are still possible since there are no active runtime systems consuming the format yet - just to be aware about that.) I think with the suggestions of Guilherme, this looks pretty good. What do you think @GuilhermeSaraiva96? |
3 participants
Add ABAP-file-format support for object type SIA6 (IAM app).
AI usage: LLMs used to create base structure and labels of the aff ABAP interface
file-formats/sia6/type/zif_aff_sia6_v1.intf.abap. Adjusted, corrected and reviewed by humans.