Skip to content

Add three types of validators#47

Open
Temerius wants to merge 7 commits into
LamoomAI:mainfrom
Temerius:main
Open

Add three types of validators#47
Temerius wants to merge 7 commits into
LamoomAI:mainfrom
Temerius:main

Conversation

@Temerius
Copy link
Copy Markdown

  1. Add method call_with_validate
  2. Add base abstract class Validator
  3. Add JSON-, XML-, YAMLValidator classes
  4. Add tests for each validator

Comment thread lamoom/prompt/lamoom.py
for error in validator.get_errors():
validation_errors.append({
"iteration": iteration,
"error": validator.format_error(error)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to add here validator's id/name

Comment thread lamoom/prompt/lamoom.py
"iteration": iteration,
"error": validator.format_error(error)
})
if validator.can_retry():
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you made that logic isolated

Comment thread lamoom/validators.py Outdated
def __init__(self, validation_format: t.Dict):
super().__init__("json", validation_format)

def validate(self, response: AIResponse):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add in AIResponse method json_from_response - which is cached property, same for yaml, xml

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean that I should add a method similar to _get_format_from_response in AIResponse, so that I can simply access the property and get the content in JSON, XML, or YAML format from response? The method should also return all found JSON, XML, and YAML.

Comment thread lamoom/validators.py Outdated
[Tag("```json", "\n```", 0), Tag("```json", "```", 0)],
start_from=0
)
if content:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in that sense, let's get all jsons, and check each json;
add another method for json_from_response, which will return an array of jsons (dict or arrays)

Copy link
Copy Markdown
Collaborator

@Katsiarynka Katsiarynka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we need to think:

  1. How the user will add to that specific prompt validators; It should be an easy way;
  2. Let's rename to call_and_validate, it's an action which will happen;
  3. Can you add a real test with JSON, Yaml, XML, we have an integrational tests;
  4. In that method - call_and_validate - we don't need to add validators; They need to be already defined to the prompts; We have defined glabal variable like PROMPTS, we can add PROMPT_VALIDATORS; - which will have all these formats;
  5. Let's delete strange schemas from required parameters;
  6. Validator should be a class, and easily be created, like for any not specialist in the programming,
  7. additional fields from the json should be passed, so we shouldn't fail if json has more fields, let's add it in the test;
  8. Add in readme a documentation for validators;
  9. AIResponse method json_from_response, jsons_from_response, ...xml, yaml;
  10. DRY (Don't Repeat Yourself)
  11. create additional ValidatorException
  12. Use functions, f.e. for iteration in range: _make_an_attempt()

Comment thread lamoom/validators.py Outdated

if content:
try:
yaml_data = yaml.safe_load(content)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY (dont repeat yourself). Let's create a class as Formatalidator, which will be a parent of each classes JSON, YAML, XML?

Comment thread tests/test_validators.py Outdated
json_format = {
"validator_type": "json",
"schema": {
"$schema": "http://json-schema.org/draft-07/schema#",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is that schema for? If you have qn, then everyone else. Be ready to make sure that everything for you makes sense, feels just stupid repetition of the doc. We can obscure that schema parameter

Comment thread lamoom/validators.py Outdated

class Validator(ABC):
def __init__(self, prompt_id: str, validator_id: str, validator_type: str, shema: t.Dict, retry_count: int = 0, retry_rules: t.Optional[t.Dict]= None):
self.prompt_id = prompt_id
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to have here prompt_id?

I believe another variable should store for prompt all validators

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants