feat: add visualization card in customized parser #16
Conversation
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
SuZhou-Joe
changed the title
There was a problem hiding this comment.
Looks like a good place to add some unit tests for the parser :)
There was a problem hiding this comment.
Yes, added cases accordingly.
| async parserProvider(interaction: Interaction) { | ||
| const additionalInfo = interaction.additional_info as { | ||
| 'VisualizationTool.output': string[]; | ||
| }; |
There was a problem hiding this comment.
This type doesn't seem right, on line 21, additionalInfo is nullable, but additional_info in Interaction is Record<string, unknown>. Is additional_info actually optional field?
There was a problem hiding this comment.
i think assertion should happen after field check
| return lines | ||
| .map((line) => line.split(',').at(column)) | ||
| .filter(<T>(v: T | null | undefined): v is T => v !== null && v !== undefined); | ||
| }; |
There was a problem hiding this comment.
could you add the TODO comment
theoretically id can contain any character, i haven't tested whether " in ids would be encoded or left as is, but current logic is not enough either way
also should output_builders/saved_objects.ts be removed? and could there be some tests similar to
There was a problem hiding this comment.
I am using csv-parser package, which is a popular and robust package to handle with csv format.
And sure, I migrate the test cases in save_objects.test.ts and add some edge cases.
| }; | ||
|
|
||
| export const VisualizationCardParser = { | ||
| id: 'core_visualization', |
There was a problem hiding this comment.
should there be an order explicitly? and i remember parsers should use the MessageParser interface?
There was a problem hiding this comment.
If order is not specified, default order value will be 999.
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
* feat: add implement visualization in customized parser Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add csv-parser lib Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add csv-parser lib Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add test cases Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: optimize Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
* feat: add implement visualization in customized parser Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add csv-parser lib Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add csv-parser lib Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add test cases Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: optimize Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Description
Parse out visualization message if the final response is generated by visualization tool.
Issues Resolved
List any issues this PR will resolve, e.g. Closes [...].
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.