Skip to content

Commit 704f62c

Browse files
committed
Address seventh review: string literal before pipe, type annotations, validate on install
- Move string literal check above pipe filter parsing so 'a | b' works - Fix type annotations: input_values list[str] | None, run_id str | None - Run validate_workflow() before installing from local path/URL - Remove duplicate string literal check from expression parser
1 parent e681ffe commit 704f62c

2 files changed

Lines changed: 19 additions & 10 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4092,7 +4092,7 @@ def extension_set_priority(
40924092
@workflow_app.command("run")
40934093
def workflow_run(
40944094
source: str = typer.Argument(..., help="Workflow ID or YAML file path"),
4095-
input_values: list[str] = typer.Option(
4095+
input_values: list[str] | None = typer.Option(
40964096
None, "--input", "-i", help="Input values as key=value pairs"
40974097
),
40984098
):
@@ -4191,7 +4191,7 @@ def workflow_resume(
41914191

41924192
@workflow_app.command("status")
41934193
def workflow_status(
4194-
run_id: str = typer.Argument(None, help="Run ID to inspect (shows all if omitted)"),
4194+
run_id: str | None = typer.Argument(None, help="Run ID to inspect (shows all if omitted)"),
41954195
):
41964196
"""Show workflow run status."""
41974197
from .workflows.engine import WorkflowEngine
@@ -4306,6 +4306,15 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None:
43064306
if not definition.id or not definition.id.strip():
43074307
console.print("[red]Error:[/red] Workflow definition has an empty or missing 'id'")
43084308
raise typer.Exit(1)
4309+
4310+
from .workflows.engine import validate_workflow
4311+
errors = validate_workflow(definition)
4312+
if errors:
4313+
console.print("[red]Error:[/red] Workflow validation failed:")
4314+
for err in errors:
4315+
console.print(f" \u2022 {err}")
4316+
raise typer.Exit(1)
4317+
43094318
dest_dir = workflows_dir / definition.id
43104319
dest_dir.mkdir(parents=True, exist_ok=True)
43114320
import shutil

src/specify_cli/workflows/expressions.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,14 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any:
118118
"""
119119
expr = expr.strip()
120120

121-
# Handle pipe filters first
121+
# String literal — check before pipes and operators so quoted strings
122+
# containing | or operator keywords are not mis-parsed.
123+
if (expr.startswith("'") and expr.endswith("'")) or (
124+
expr.startswith('"') and expr.endswith('"')
125+
):
126+
return expr[1:-1]
127+
128+
# Handle pipe filters
122129
if "|" in expr:
123130
parts = expr.split("|", 1)
124131
value = _evaluate_simple_expression(parts[0].strip(), namespace)
@@ -143,13 +150,6 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any:
143150
return _filter_default(value)
144151
return value
145152

146-
# String literal — check before operators so quoted strings
147-
# containing operator keywords (e.g. 'a in b') are not mis-parsed.
148-
if (expr.startswith("'") and expr.endswith("'")) or (
149-
expr.startswith('"') and expr.endswith('"')
150-
):
151-
return expr[1:-1]
152-
153153
# Boolean operators — parse 'or' first (lower precedence) so that
154154
# 'a or b and c' is evaluated as 'a or (b and c)'.
155155
if " or " in expr:

0 commit comments

Comments
 (0)