[ATO-1315] Flows yaml schema #12901

Urkem · 2023-10-06T14:47:28Z

Proposed changes:

ATO-1315

Status (please check what you already did):

added some tests for the functionality
updated the documentation
updated the changelog (please check changelog for instructions)
reformat files using black (please check Readme for instructions)

…validation

ancalita

Looks great 💯 I'd advise to open a companion PR in financial llm demo bot pinning rasa to this branch to test your PR 🙏🏻

rasa/shared/utils/validation.py

rasa/shared/core/flows/flows_yaml_schema.json

Urkem · 2023-10-10T11:34:58Z

@ancalita The validation itself seems to fail on financial llm demo bot... (the flows got loaded. Still not sure if the flow name is a required field for a flow)

The training works if I do skip validation (this does not skip checking the schema):

ancalita · 2023-10-10T11:42:59Z

The validation itself seems to fail on financial llm demo bot

I think that's because pattern_code_change is missing a name, I think this should be part of the schema and should be mandatory, in which case schema validation should fail.

Urkem · 2023-10-10T13:24:38Z

I think that's because pattern_code_change is missing a name, I think this should be part of the schema and should be mandatory, in which case schema validation should fail.

Added name as required and pattern_code_change is a part of OSS so I updated it accordingly.

twerkmeister

Overall looks good to me. Good job on the json schema!

One remaining question regarding flow names being required... In my recent PR I added a default for the name attribute of flow that basically converts the id and removes the underscores from it to form a name if none is present. In the names you added this is exactly the same pattern. I find it quite redundant to always duplicate the id in the name. I think we can go with the name = id (with underscores replaced) default and then people can adjust the name if they still want to.

…validation

tests/test_validator.py

ancalita

Awesome 🙌🏻 I left a few questions, I'd add some more tests to check for each step type and potential values given to each step's different properties (valid and invalid values). Also I'd add another test for testing whether random keys can be added to a flow or flow steps.

rasa/shared/core/flows/flows_yaml_schema.json

tests/shared/utils/test_validation.py

rasa/shared/core/flows/flows_yaml_schema.json

ancalita

LGTM 💯 Approving to unblock you, but on condition that the passing e2e tests for demo bot are all good 😄 I also left 2 questions for clarification.

ancalita · 2023-10-13T11:21:53Z

rasa/shared/core/flows/flows_yaml_schema.json

+          },
+          {
+            "required": [
+              "intent"


Just double-checking if this is still in use yet, or if it's been discarded, i thought we removed the need for steps of type user message, unless this is something different and related to coexistence?

Added because of the flow spec specifically 3. Flow Rules 3rd point it's not used anywhere (that I found) same with nlu_trigger maybe they are used for same thing...

Ok, yeah I was also under the impression that we don't use the intent step any longer, but nlu_trigger might be useful in coexistence. Best to check with Daksh/Engine on Slack if this part of the schema is correct.

Yes we use nlu_trigger now. Although that property hasn't been implemented yet. @twerkmeister would know the implementation details of this.

Thanks for the clarification @dakshvar22 removed the intent step from the schema, the nlu_trigger is already part of the schema here (structure is the same as in the flows spec)

rasa/shared/core/flows/flows_yaml_schema.json

Urkem · 2023-10-13T12:58:33Z

@twerkmeister @ancalita running the e2e tests on the demo bot (with the default flow change and without the change) is 50pass 18fail always is that the expected ratio or do all tests pass?

Rasa Version      :         3.8.0a11
Minimum Compatible Version: 3.5.0
Rasa SDK Version  :         3.8.0a1
Python Version    :         3.10.10
Operating System  :         macOS-13.6-arm64-arm-64bit
Python Path       :         /Users/urosmilovanovic/.pyenv/versions/3.10.10/envs/dm2-1-demo/bin/python
Rasa Plus Version  :         3.8.0a11

github-actions · 2023-10-13T18:11:17Z

🚀 A preview of the docs have been deployed at the following URL: https://12901--rasahq-docs-rasa-v2.netlify.app/docs/rasa

sonarqubecloud · 2023-10-13T18:29:09Z

Kudos, SonarCloud Quality Gate passed!

5 Bugs
0 Vulnerabilities
1 Security Hotspot
49 Code Smells

0.0% Coverage
0.9% Duplication

The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

ancalita · 2023-10-16T08:33:01Z

@twerkmeister @ancalita running the e2e tests on the demo bot (with the default flow change and without the change) is 50pass 18fail always is that the expected ratio or do all tests pass?

Hey @Urkem Apologies, I missed this message on Friday, if the same tests fail without the schema change, I'd advise to open a ticket with the details about failing tests and alert Dan about it, should be investigated what change caused those tests to fail since the tests labelled as passing always passed (68 out of 68).

Urkem added 2 commits September 26, 2023 17:14

init flows_yaml_schema

92a0e24

add basic schema

9cb1b04

Urkem requested a review from ancalita October 6, 2023 14:50

Urkem added 5 commits October 9, 2023 12:09

Merge remote-tracking branch 'origin/dm2' into ATO-1315-improve-yaml-…

06839c5

…validation

add fail tests

3a04dd8

add changelog

bb4f625

fix minProp

e628f4b

add next to set slot

994ec5f

Urkem marked this pull request as ready for review October 9, 2023 13:50

Urkem requested a review from a team as a code owner October 9, 2023 13:50

Urkem requested review from a team and twerkmeister and removed request for a team October 9, 2023 13:52

Urkem added 4 commits October 9, 2023 16:14

fix set_slots test

d966b24

rename set_slot to set_slots

f685d8e

add intent

01372e6

Trigger CI

2175ab6

ancalita reviewed Oct 10, 2023

View reviewed changes

rasa/shared/utils/validation.py Outdated Show resolved Hide resolved

rasa/shared/core/flows/flows_yaml_schema.json Outdated Show resolved Hide resolved

Urkem added 2 commits October 10, 2023 12:48

add entry_prompt

6351f72

add null to slot values

f5f65dd

Urkem added 2 commits October 10, 2023 13:54

remove docs link

5a3bf1c

fix lint

6627fb7

Urkem requested a review from ancalita October 10, 2023 13:09

twerkmeister reviewed Oct 12, 2023

View reviewed changes

fix read

47c596c

Urkem force-pushed the ATO-1315-improve-yaml-validation branch from 8ef2870 to 47c596c Compare October 12, 2023 11:33

drop reset in set_slots

bfea853

Urkem added 3 commits October 12, 2023 14:03

add name to defaults

cddceab

Merge remote-tracking branch 'origin/dm2' into ATO-1315-improve-yaml-…

226d193

…validation

fix test

8aebb5f

Urkem commented Oct 12, 2023

View reviewed changes

tests/test_validator.py Outdated Show resolved Hide resolved

Urkem requested a review from twerkmeister October 12, 2023 13:28

ancalita reviewed Oct 12, 2023

View reviewed changes

rasa/shared/core/flows/flows_yaml_schema.json Outdated Show resolved Hide resolved

rasa/shared/core/flows/flows_yaml_schema.json Show resolved Hide resolved

rasa/shared/core/flows/flows_yaml_schema.json Outdated Show resolved Hide resolved

add additional slot types

9492b81

ancalita reviewed Oct 12, 2023

View reviewed changes

tests/shared/utils/test_validation.py Outdated Show resolved Hide resolved

fix lint

a49870d

Urkem requested a review from ancalita October 12, 2023 18:14

Urkem added 2 commits October 12, 2023 20:15

typo

dbde8bf

remove just id step

da63faf

Urkem commented Oct 13, 2023

View reviewed changes

rasa/shared/core/flows/flows_yaml_schema.json Show resolved Hide resolved

Urkem added 3 commits October 13, 2023 11:03

add next to schemas

54e613f

enforce next to none for link steps

2448325

fix fail messages

cd07f27

ancalita approved these changes Oct 13, 2023

View reviewed changes

Urkem added 3 commits October 13, 2023 19:32

remove intent step

61a9891

fix tests

5b21911

fix lint

72d50dc

Urkem merged commit 463c703 into dm2 Oct 16, 2023
97 checks passed

Urkem deleted the ATO-1315-improve-yaml-validation branch October 16, 2023 07:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ATO-1315] Flows yaml schema #12901

[ATO-1315] Flows yaml schema #12901

Urkem commented Oct 6, 2023 •

edited

Loading

ancalita left a comment

Urkem commented Oct 10, 2023

ancalita commented Oct 10, 2023

Urkem commented Oct 10, 2023 •

edited

Loading

twerkmeister left a comment

ancalita left a comment

ancalita left a comment

ancalita Oct 13, 2023

Urkem Oct 13, 2023 •

edited

Loading

ancalita Oct 13, 2023

dakshvar22 Oct 13, 2023

Urkem Oct 13, 2023 •

edited

Loading

Urkem commented Oct 13, 2023

github-actions bot commented Oct 13, 2023

sonarqubecloud bot commented Oct 13, 2023

ancalita commented Oct 16, 2023

[ATO-1315] Flows yaml schema #12901

[ATO-1315] Flows yaml schema #12901

Conversation

Urkem commented Oct 6, 2023 • edited Loading

ancalita left a comment

Choose a reason for hiding this comment

Urkem commented Oct 10, 2023

ancalita commented Oct 10, 2023

Urkem commented Oct 10, 2023 • edited Loading

twerkmeister left a comment

Choose a reason for hiding this comment

ancalita left a comment

Choose a reason for hiding this comment

ancalita left a comment

Choose a reason for hiding this comment

ancalita Oct 13, 2023

Choose a reason for hiding this comment

Urkem Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

ancalita Oct 13, 2023

Choose a reason for hiding this comment

dakshvar22 Oct 13, 2023

Choose a reason for hiding this comment

Urkem Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

Urkem commented Oct 13, 2023

github-actions bot commented Oct 13, 2023

sonarqubecloud bot commented Oct 13, 2023

ancalita commented Oct 16, 2023

Urkem commented Oct 6, 2023 •

edited

Loading

Urkem commented Oct 10, 2023 •

edited

Loading

Urkem Oct 13, 2023 •

edited

Loading

Urkem Oct 13, 2023 •

edited

Loading