new validate command tests added; validate feature fixes and updates

prich/cli/validate.py

@@ -6,7 +6,7 @@
 from pydantic import ValidationError as PydanticValidationError
 from prich.constants import PRICH_DIR_NAME
 from prich.models.template import CommandStep, PythonStep
-from prich.core.file_scope import classify_path
+from prich.core.file_scope import classify_path, normalize_path
 from prich.core.loaders import find_template_files, load_template_model, get_env_vars, _load_yaml
 from prich.core.utils import console_print, shorten_path, get_prich_dir, is_just_filename, get_cwd_dir, get_home_dir
 
@@ -21,14 +21,16 @@ def template_model_doctor(template_yaml: dict, model_load_error: PydanticValidat
         if template_yaml and err.get("loc"):
             # hide extra layering
             if err.get("loc")[0] == "steps":
-                if err.get("loc")[2] in ['llm', 'command', 'python', 'render']:
+                if len(err.get("loc")) >= 3 and err.get("loc")[2] in ['llm', 'command', 'python', 'render']:
                     details = err.get("loc")[2]
                     err['loc'] = tuple(err.get("loc")[:2] + err.get("loc")[3:])
 
             trace_dir = template_yaml.copy()
             loc_list = err.get("loc")[:-1] if len(err.get("loc")) > 1 else err.get("loc")
             for x in loc_list:
                 try:
+                    if trace_dir[x] is None:
+                        break
                     trace_dir = trace_dir[x]
                 except Exception:
                     break
@@ -45,8 +47,10 @@ def template_model_doctor(template_yaml: dict, model_load_error: PydanticValidat
                 template_overview = re.sub("(\\.\\.\\.)", f"[yellow]+{err.get('loc')[-1]}: ...[/yellow]\n\\1",
                                            template_overview, count=1)
             else:
+                if 'Input tag ' in err.get('msg'):
+                    err["msg"] = err['msg'].replace("Input tag ", "Field value ").replace(" any of the expected tags", " any of the expected values")
                 if 'Input should be' in err.get('msg'):
-                    err["msg"] = err['msg'].replace("Input should be", "Field value should be")
+                    err["msg"] = err['msg'].replace("Input should be ", "Field value should be ")
                 highligh_block = err.get('loc')[-1] if isinstance(err.get('loc')[-1], str) else err.get('loc')[-2]
                 template_overview = re.sub(f"([\n]*(?:\\s+)|^)({highligh_block})(:)", "\\1[red]\\2[/red]\\3",
                                            template_overview, count=1)
@@ -92,7 +96,7 @@ def template_model_doctor(template_yaml: dict, model_load_error: PydanticValidat
             else:
                 doc = "See Template Content Documentation https://oleks-dev.github.io/prich/reference/template/content/"
             err_loc_string = re.sub(f"({err.get('loc')[-1]})$", "[red]\\1[/red]", err_loc_string)
-            found_issues_list.append(f"""{len(found_issues_list)+1}. [red]{err.get('msg')}[/red] '[white]{err_loc_string}[/white]':\n[white]{template_overview}[/white]{doc}""")
+            found_issues_list.append(f"""{len(found_issues_list)+1}. [red]{err.get('msg')}[/red] at '[white]{err_loc_string}[/white]':\n[white]{template_overview}[/white]{doc}""")
     return found_issues_list
 
 
@@ -111,7 +115,10 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool,
     if validate_file and (global_only or local_only or template_id):
         raise click.ClickException(f"When YAML file is selected it doesn't combine with local, global, or id options, use: 'prich validate --file ./{PRICH_DIR_NAME}/templates/test-template/test-template.yaml'")
 
-    if validate_file and not validate_file.exists():
+    if validate_file:
+        validate_file = normalize_path(validate_file, cwd=get_cwd_dir())
+
+    if validate_file and (not validate_file.exists() or not validate_file.is_file()):
         raise click.ClickException(f"Failed to find {validate_file} template file.")
 
     # Load Template Files
@@ -154,10 +161,9 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool,
         model_failures_count = 0
         output = []
         try:
-            if template_file.is_file():
-                template_yaml = _load_yaml(template_file)
-                template_id = template_yaml.get("id") if template_yaml else None
-                template_name = template_yaml.get("name") if template_yaml else None
+            template_yaml = _load_yaml(template_file)
+            template_id = template_yaml.get("id") if template_yaml else None
+            template_name = template_yaml.get("name") if template_yaml else None
             try:
                 template = load_template_model(template_file)
             except PydanticValidationError as e:
@@ -169,8 +175,18 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool,
                 raise click.ClickException(f"1. [red]{str(e)}[/red]")
             if template.venv in ["isolated", "shared"]:
                 venv_folder = (Path(template.folder) / "scripts") if template.venv == "isolated" else get_prich_dir() / "venv"
+                python_steps = [step for step in template.steps if step.type == 'python']
+                if not python_steps:
+                    extra_note = ". There are no steps with type 'python' found, if python is not used you can remove the 'venv' parameter from the template"
+                else:
+                    extra_note = ""
+                if template.venv == 'isolated':
+                    installation_note = f" Install it by running 'prich venv-install {template.id}'."
+                else:
+                    # TODO: introduce help for shared venv installation
+                    installation_note = ""
                 if not venv_folder.exists():
-                    failures_list.append(f"{len(failures_list)+1}. [red]Failed to find {template.venv} venv at {shorten_path(str(venv_folder))}.[/red] Install it by running 'prich venv-install {template.id}'.")
+                    failures_list.append(f"{len(failures_list)+1}. [red]Failed to find {template.venv} venv at {shorten_path(str(venv_folder))}{extra_note}.[/red]{installation_note}")
             idx = 0
             for step in template.steps:
                 idx += 1
@@ -196,7 +212,7 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool,
             output.append(f"- {template.id} [dim]({template.source.value}) {shorten_path(str(template_file))}[/dim]: ")
             if len(failures_list) > 0:
                 failures_found = True
-                output[-1] += f"[red]is not valid[/red] ({len(failures_list)} issues)"
+                output[-1] += f"[red]is not valid[/red] ({len(failures_list)} issue{'s' if len(failures_list)>1 else ''})"
                 failures = '  ' + '\n  '.join(failures_list)
                 output.append(failures)
                 output.append("")
@@ -206,11 +222,11 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool,
             failures_found = True
             template_source = classify_path(template_file)
             error_lines = '  ' + '\n  '.join([x for x in e.message.split('\n')]) + '\n'
-            output.append(f"""- {f"{template_id} " if template_id else ''}[dim]({template_source.value}) {shorten_path(str(template_file))}[/dim]: [red]is not valid[/red] {f'({model_failures_count} issues)' if model_failures_count > 0 else '(1 issue)'}\n  [red]Failed to load template{f" {template_id}" if template_id else ""}{f" ({template_name})" if template_name else ""}[/red]:\n{error_lines}""")
+            output.append(f"""- {f"{template_id} " if template_id else ''}[dim]({template_source.value}) {shorten_path(str(template_file))}[/dim]: [red]is not valid[/red] {f'({model_failures_count} issue{"s" if model_failures_count > 1 else ""})' if model_failures_count > 0 else '(1 issue)'}\n  [red]Failed to load template{f" {template_id}" if template_id else ""}{f" ({template_name})" if template_name else ""}[/red]:\n{error_lines}""")
         except Exception as e:
             failures_found = True
             template_source = classify_path(template_file)
-            output.append(f"""- {f"{template_id} " if template_id else ''}[dim]({template_source.value}) {shorten_path(str(template_file))}[/dim]: [red]is not valid[/red] (1 issue)\n  [red]Failed to load template{f" {template_id}" if template_id else ""}{f" ({template_name})" if template_name else ""}[/red]:\n  {str(e)}""")
+            output.append(f"""- {f"{template_id} " if template_id else ''}[dim]({template_source.value}) {shorten_path(str(template_file))}[/dim]: [red]is not valid[/red] (1 issue)\n  [red]Failed to load template{f" {template_id}" if template_id else ""}{f" ({template_name})" if template_name else ""}[/red]:\n  1. {str(e)}""")
         if (invalid_only and failures_found) or not invalid_only:
             console_print('\n'.join(output))
         if failures_found:

prich/core/file_scope.py

@@ -6,7 +6,7 @@
 from prich.models.file_scope import FileScope
 
 
-def _normalize(p: Path, *, cwd: Path) -> Path:
+def normalize_path(p: Path, *, cwd: Path) -> Path:
     """
     Expand ~, make absolute relative to cwd, and resolve as much as possible.
     Works for non-existent paths too (resolves the existing parent).
@@ -66,9 +66,9 @@ def classify_path(
     global_root = home / PRICH_DIR_NAME
 
     if follow_symlinks:
-        p = _normalize(file, cwd=cwd)
-        lr = _normalize(local_root, cwd=cwd)
-        gr = _normalize(global_root, cwd=cwd)
+        p = normalize_path(file, cwd=cwd)
+        lr = normalize_path(local_root, cwd=cwd)
+        gr = normalize_path(global_root, cwd=cwd)
     else:
         # Don't resolve symlinks; still expand and absolutize
         p = (cwd / Path(file).expanduser()) if not Path(file).expanduser().is_absolute() else Path(file).expanduser()

prich/core/loaders.py

@@ -15,7 +15,7 @@
 
 def _load_yaml(path: Path) -> Dict:
     import yaml
-    if not path.exists():
+    if not path.exists() or not path.is_file():
         return {}
     with path.open("r", encoding="utf-8") as f:
         return yaml.safe_load(f) or {}

prich/core/utils.py

@@ -113,7 +113,7 @@ def get_prich_templates_dir(global_only: bool = None) -> Path:
 
 def shorten_path(path: str | Path) -> str:
     """ Return short path using ~/... or ./... instead of a full absolute path """
-    home = str(Path.home())
+    home = str(get_home_dir())
     cwd = str(get_cwd_dir())
     if isinstance(path, Path):
         path = str(path)

tests/resources/no_id.yaml

@@ -0,0 +1,11 @@
+name: Test Template
+version: '1.0'
+description: Example template
+tags:
+- example
+- writer
+steps:
+- name: Ask to generate text
+  type: llm
+  input: Generate short phrase
+schema_version: '1.0'

tests/resources/no_name.yaml

@@ -0,0 +1,8 @@
+id: test-template
+version: '1.0'
+description: Example template
+steps:
+- name: Ask to generate text
+  type: llm
+  input: Generate short phrase
+schema_version: '1.0'

tests/resources/no_schema_version.yaml

@@ -0,0 +1,2 @@
+id: "test"
+name: "test"

tests/resources/no_shared_venv.yaml

@@ -0,0 +1,11 @@
+id: test-template
+name: Test Template
+version: '1.0'
+description: Example template - Generate text about specified topic
+venv: "shared"
+steps:
+- name: Ask to generate text
+  type: llm
+  instructions: You are {{ role }}
+  input: Generate text about {{ topic }}
+schema_version: '1.0'

tests/resources/no_steps.yaml

@@ -0,0 +1,6 @@
+id: test-template
+name: test template
+version: '1.0'
+description: Example template
+steps:
+schema_version: '1.0'

tests/resources/no_venv.yaml

@@ -0,0 +1,11 @@
+id: test-template
+name: Test Template
+version: '1.0'
+description: Example template - Generate text about specified topic
+venv: "isolated"
+steps:
+- name: Ask to generate text
+  type: llm
+  instructions: You are {{ role }}
+  input: Generate text about {{ topic }}
+schema_version: '1.0'

tests/resources/not_supported_schema.yaml

@@ -0,0 +1,3 @@
+id: "test"
+name: "test"
+schema_version: "0.1"

tests/resources/wrong_steps.yaml

@@ -0,0 +1,15 @@
+id: test-template
+name: test template
+version: '1.0'
+description: Example template
+steps:
+- name: Ask to generate 1st text
+  type: provider
+  input: Generate short phrase
+- step_name: Ask to generate 2nd text
+  type: llm
+  input: Generate short phrase
+- name: Ask to generate 3nd text
+  type: llm
+  input: Generate short phrase
+schema_version: '1.0'

tests/resources/wrong_variables.yaml

@@ -0,0 +1,28 @@
+id: test-template
+name: test template
+version: '1.0'
+description: Example template
+steps:
+- name: Ask to generate 3nd text
+  type: llm
+  input: You are an engineer
+variables:
+  - var_name: "test"
+  - name: "test-variable"
+  - name: "test_variable"
+    type: "integer"
+  - name: "test_variable"
+    required: "hello"
+  - name: "test_variable"
+    default: null
+  - name: "test_variable"
+    cli_option: "test-var"
+
+# - name: role
+    #  type: str
+    #  description: Role of the Assistant
+    #  default: article writer
+    #  required: false
+    #  cli_option: --role
+
+schema_version: '1.0'

tests/resources/yaml_error.yaml

@@ -0,0 +1,10 @@
+id: test-template
+name: Test Template
+version: '1.0'
+description_ Example template - Generate text about specified topic
+steps:
+- name: Ask to generate text
+  type: llm
+  instructions: You are {{ role }}
+  input: Generate text about {{ topic }}
+schema_version: '1.0'

tests/test_template_steps.py

@@ -5,7 +5,6 @@
 from prich.models.config import SettingsConfig, ProviderModeModel
 from prich.models.template import TemplateModel, LLMStep
 from prich.models.config import ConfigModel
-from tests.fixtures.paths import mock_paths
 from tests.utils.utils import capture_stdout
 
 sample_variables = {
@@ -321,7 +320,7 @@ def test_step_render_template(monkeypatch, template_string, expected, expected_e
      },
 ]
 @pytest.mark.parametrize("case", get_step_send_to_llm_CASES, ids=[c["id"] for c in get_step_send_to_llm_CASES])
-def test_step_send_to_llm(tmp_path, mock_paths, case, monkeypatch):
+def test_step_send_to_llm(tmp_path, case, monkeypatch):
     from prich.core.steps.step_send_to_llm import send_to_llm
 
     if case.get("expected_exception"):

tests/test_validate.py

@@ -1,3 +1,4 @@
+import os
 from pathlib import Path
 import pytest
 from click.testing import CliRunner
@@ -19,7 +20,12 @@
     {"id": "file_with_template_id_param", "args": ["--file", "test.yaml", "--id", "test-template"],
      "expected_output": "When YAML file is selected it doesn't combine with local, global, or id options"},
     {"id": "file_not_present", "args": ["--file", "test.yaml"],
-     "expected_output": "Failed to find test.yaml template file."},
+     "expected_output": ["Failed to find", "test.yaml template file."]},
+    {"id": "path_as_file", "args": ["--file", "."],
+     "expected_output": ["Failed to find", " template file."]},
+    {"id": "wrong_template_from_file", "add_wrong_template": True,
+     "args": ["--file", "./.prich/templates/tpl-local-wrong/tpl-local-wrong.yaml"],
+     "expected_output": "tpl-local-wrong.yaml: is not valid"},
     {"id": "no_templates_found", "args": [],
      "expected_output": "No Templates found."},
     {"id": "no_template_if_found", "args": ["--id", "test-template"],
@@ -81,6 +87,66 @@
          "Failed to find call python file ./.prich/templates/template-local/scripts/echo1.py",
          "Failed to find call python file ./.prich/templates/template-local/scripts/echo2.py",
      ]},
+
+    # wrong templates from resources
+    {"id": "file_empty",
+     "args": ["--file", "{resources}/empty.yaml"],
+     "expected_output": ["empty.yaml: is not valid", "check if file or contents are correct"]},
+    {"id": "file_no_schema_version",
+     "args": ["--file", "{resources}/no_schema_version.yaml"],
+     "expected_output": ["no_schema_version.yaml", "is not valid", "Failed to load template",
+                         "Unsupported template schema version NOT SET"]},
+    {"id": "file_not_supported_schema",
+     "args": ["--file", "{resources}/not_supported_schema.yaml"],
+     "expected_output": ["not_supported_schema.yaml", "is not valid", "Failed to load template",
+                         "Unsupported template schema version 0.1"]},
+    {"id": "file_no_id",
+     "args": ["--file", "{resources}/no_id.yaml"],
+     "expected_output": ["no_id.yaml", "is not valid (1 issue)", "Failed to load template",
+                         "Missing required field at 'id'", "+id: ..."]},
+    {"id": "file_no_name",
+     "args": ["--file", "{resources}/no_name.yaml"],
+     "expected_output": ["no_name.yaml", "is not valid", "Failed to load template",
+                         "Missing required field at 'name'", "+name: ..."]},
+    {"id": "file_no_steps",
+     "args": ["--file", "{resources}/no_steps.yaml"],
+     "expected_output": ["no_steps.yaml", "is not valid", "Failed to load template",
+                         "Field value should be a valid list at 'steps'", "steps: null",
+                         "See Steps documentation:", "https://oleks-dev.github.io/prich/reference/template/steps/"]},
+    {"id": "file_wrong_steps",
+     "args": ["--file", "{resources}/wrong_steps.yaml"],
+     "expected_output": ["wrong_steps.yaml", "is not valid (3 issues)", "Failed to load template",
+                         "1. Field value 'provider' found using 'type' does not match any of the expected values: 'python', 'command', 'llm', 'render' at 'steps[1]':  ---  - name: Ask to generate 1st",
+                         "2. Missing required field at 'steps[2].name':  ---  step_name: Ask to generate 2nd",
+                         "3. Unrecognized field at 'steps[2].step_name':  ---  step_name: Ask to generate 2nd",
+                         "See Steps documentation:", "https://oleks-dev.github.io/prich/reference/template/steps/"]},
+    {"id": "file_wrong_variables",
+     "args": ["--file", "{resources}/wrong_variables.yaml"],
+     "expected_output": ["wrong_variables.yaml", "is not valid (4 issues)", "Failed to load template",
+                         "1. Missing required field at 'variables[1].name':  ---  var_name: test  +name: ...",
+                         "2. Unrecognized field at 'variables[1].var_name':  ---  var_name: test  ... ",
+                         "3. Field value should be 'str',",
+                         "4. Field value should be a valid boolean, unable to interpret input at 'variables[4].required'",
+                         "See Variables documentation https://oleks-dev.github.io/prich/reference/template/variables/"]},
+    {"id": "file_no_python_not_found_isolated_venv",
+     "args": ["--file", "{resources}/no_venv.yaml"],
+     "expected_output": ["no_venv.yaml", "is not valid (1 issue)",
+                         "1. Failed to find isolated venv at",
+                         "There are no steps with type 'python' found",
+                         "Install it by running 'prich venv-install test-template'."
+     ]},
+    {"id": "file_no_python_not_found_shared_venv",
+     "args": ["--file", "{resources}/no_shared_venv.yaml"],
+     "expected_output": ["no_shared_venv.yaml", "is not valid (1 issue)",
+                         "1. Failed to find shared venv at",
+                         "There are no steps with type 'python' found",
+     ]},
+    {"id": "file_yaml_error",
+     "args": ["--file", "{resources}/yaml_error.yaml"],
+     "expected_output": ["yaml_error.yaml", "is not valid (1 issue)",
+                         "Failed to load template:", "1. while scanning a simple key",
+                         "could not find expected ':'",
+     ]},
 ]
 @pytest.mark.parametrize("case", get_validate_template_CASES, ids=[c["id"] for c in get_validate_template_CASES])
 def test_validate_template(mock_paths, monkeypatch, case, template, basic_config):
@@ -125,9 +191,14 @@ def test_validate_template(mock_paths, monkeypatch, case, template, basic_config
 
     runner = CliRunner()
     with runner.isolated_filesystem(temp_dir=mock_paths.home_dir):
-        result = runner.invoke(validate_templates, case.get("args"))
+        resources_path = os.path.dirname(__file__)
+        args = []
+        for arg in case.get("args"):
+            args.append(arg.replace("{resources}", str(Path(resources_path) / "resources")))
+
+        result = runner.invoke(validate_templates, args)
         if case.get("expected_output") is not None:
             if isinstance(case.get("expected_output"), str):
                 case["expected_output"] = [case.get("expected_output")]
             for expected_output in case.get("expected_output"):
-                assert expected_output in result.output.replace("\n", "")
+                assert expected_output in result.output.replace("\n", " ").replace("  ", " ")