Deprecate now() in favour of execution.timestamp (#104)

docs/template_globals.md

@@ -54,7 +54,7 @@ validate`, exit 2).
 | `include_file` | 1 to 2 | The contents of a file, read when the step runs. |
 | `loop_item_at` | 2 | An item at a fixed index of a named loop: `loop_item_at('outer', 0)`. |
 | `loop_var` | 2 | A named loop's variable by name: `loop_var('outer', 'item')`. |
-| `now` | 0 | The current time. Re-evaluated at every use, so two steps in one run disagree -- prefer `execution.timestamp` where a run needs one answer. |
+| `now` | 0 | The current time, read afresh at every use, so two steps of one run disagree. Deprecated: use `execution.timestamp`, which is the same for every step of a run. |
 
 A name that is not on this list is not a global. `{{ nowx() }}` is a
 typo and is refused at compile time rather than becoming an undefined

examples/data_processing_pipeline.yaml

@@ -237,7 +237,7 @@ steps:
           "errors": {{ validate_schema.errors | default([]) | to_json }},
           "warnings": {{ validate_schema.warnings | default([]) | to_json }},
           "rows_validated": {{ validate_schema.rows_validated | default(0) }},
-          "timestamp": "{{ now() }}",
+          "timestamp": "{{ execution.timestamp }}",
           "input_file": "{{ input_file }}",
           "validation_mode": "{{ validate_schema.mode | default('strict') }}"
         }
@@ -370,7 +370,7 @@ steps:
 
         ---
 
-        *Report generated on: {{ now() }}*
+        *Report generated on: {{ execution.timestamp }}*
         *Pipeline ID: {{ pipeline_id }}*
     dependencies:
       - clean_data

examples/enhanced/data_processing_pipeline_enhanced.yaml

@@ -235,7 +235,7 @@ steps:
       \ | to_json }},\n  \"errors\": {{ validate_schema.errors | default([]) | to_json\
       \ }},\n  \"warnings\": {{ validate_schema.warnings | default([]) | to_json }},\n\
       \  \"rows_validated\": {{ validate_schema.rows_validated | default(0) }},\n\
-      \  \"timestamp\": \"{{ now() }}\",\n  \"input_file\": \"{{ input_file }}\",\n\
+      \  \"timestamp\": \"{{ execution.timestamp }}\",\n  \"input_file\": \"{{ input_file }}\",\n\
       \  \"validation_mode\": \"{{ validate_schema.mode | default('strict') }}\"\n\
       }\n"
   dependencies:
@@ -312,7 +312,7 @@ steps:
       \ %}\n*Statistical analysis pending*\n{% endif %}\n\n## Recommendations\n\n\
       {% if quality_check.result.recommendations %}\n{% for rec in quality_check.result.recommendations\
       \ %}\n- \U0001F4CC {{ rec }}\n{% endfor %}\n{% endif %}\n\n---\n\n*Report generated\
-      \ on: {{ now() }}*\n*Pipeline ID: {{ pipeline_id }}*\n"
+      \ on: {{ execution.timestamp }}*\n*Pipeline ID: {{ pipeline_id }}*\n"
   dependencies:
   - clean_data
   - quality_check

examples/enhanced/interactive_pipeline_enhanced.yaml

@@ -318,7 +318,7 @@ steps:
 
       ## Timestamp
 
-      Generated at: {{ now() }}
+      Generated at: {{ execution.timestamp }}
 
       '
   dependencies:

examples/enhanced/validation_pipeline_enhanced.yaml

@@ -51,7 +51,7 @@ steps:
   parameters:
     path: examples/outputs/validation_pipeline/reports/validation_report.json
     content: "{\n  \"validation_result\": {{ validate_data | to_json }},\n  \"extracted_data\"\
-      : {{ extract_info | to_json }},\n  \"timestamp\": \"{{ now() }}\"\n}\n"
+      : {{ extract_info | to_json }},\n  \"timestamp\": \"{{ execution.timestamp }}\"\n}\n"
   dependencies:
   - validate_data
   - extract_info

examples/interactive_pipeline.yaml

@@ -315,6 +315,6 @@ steps:
         - **Processing Useful**: {{ 'Yes' if collect_feedback.summary.boolean_summary.processing_useful else 'No' }}
 
         ## Timestamp
-        Generated at: {{ now() }}
+        Generated at: {{ execution.timestamp }}
     dependencies:
       - collect_feedback

examples/validation_pipeline.yaml

@@ -59,7 +59,7 @@ steps:
         {
           "validation_result": {{ validate_data | to_json }},
           "extracted_data": {{ extract_info | to_json }},
-          "timestamp": "{{ now() }}"
+          "timestamp": "{{ execution.timestamp }}"
         }
     dependencies:
       - validate_data

src/orchestrator/core/template_globals.py

@@ -50,6 +50,10 @@ class GlobalSpec:
     min_args: int
     max_args: Optional[int]  # None means unbounded
     summary: str
+    #: What to write instead, if this global should no longer be used. The
+    #: call keeps working -- pipelines in the wild use it -- but validation
+    #: says so, once, with the replacement named.
+    deprecated_for: Optional[str] = None
 
     def accepts(self, positional: int) -> bool:
         if positional < self.min_args:
@@ -71,8 +75,10 @@ def arity(self) -> str:
 GLOBAL_SPECS: Tuple[GlobalSpec, ...] = (
     GlobalSpec(
         "now", 0, 0,
-        "The current time. Re-evaluated at every use, so two steps in one run "
-        "disagree -- prefer `execution.timestamp` where a run needs one answer.",
+        "The current time, read afresh at every use, so two steps of one run "
+        "disagree. Deprecated: use `execution.timestamp`, which is the same "
+        "for every step of a run.",
+        deprecated_for="execution.timestamp",
     ),
     GlobalSpec(
         "file_exists", 1, 2,
@@ -113,6 +119,7 @@ def global_spec(name: str) -> Optional[GlobalSpec]:
 #: on these rather than on message text.
 NOT_CALLED = "global_not_called"
 WRONG_ARITY = "global_wrong_arity"
+DEPRECATED = "global_deprecated"
 
 
 @dataclass(frozen=True)
@@ -123,6 +130,8 @@ class GlobalMisuse:
     code: str
     message: str
     suggestion: str
+    #: "error" refuses the pipeline; "warning" lets it run and says so.
+    severity: str = "error"
 
 
 def find_global_misuse(ast: Any) -> List[GlobalMisuse]:
@@ -202,5 +211,25 @@ def find_global_misuse(ast: Any) -> List[GlobalMisuse]:
                 ),
                 suggestion=f"{spec.name} expects {spec.arity} argument(s)",
             ))
+            continue
+
+        # A correct call to something that should no longer be written. Not an
+        # error: pipelines in the wild use it and must keep running.
+        if spec.deprecated_for is not None:
+            key = (spec.name, DEPRECATED)
+            if key in seen:
+                continue
+            seen.add(key)
+            misuse.append(GlobalMisuse(
+                name=spec.name,
+                code=DEPRECATED,
+                message=(
+                    f"'{spec.name}()' is deprecated: it is read afresh at every "
+                    f"use, so two steps of one run disagree. Use "
+                    f"'{spec.deprecated_for}', which is the same for every step."
+                ),
+                suggestion=spec.deprecated_for,
+                severity="warning",
+            ))
 
     return misuse

src/orchestrator/validation/template_validator.py

@@ -335,13 +335,20 @@ def _validate_variables(
             # nothing fails. The AST is what tells a call apart from an
             # attribute access or a bare mention; the text does not.
             for misuse in find_global_misuse(ast):
-                errors.append(TemplateValidationError(
+                reported = TemplateValidationError(
                     template=template,
                     error_type=misuse.code,
                     message=misuse.message,
                     context_path=context_path,
                     suggestions=[misuse.suggestion],
-                ))
+                    severity=misuse.severity,
+                )
+                # A deprecated global still works, so saying so must not stop
+                # the pipeline; a misused one cannot work, so it must.
+                if misuse.severity == "error":
+                    errors.append(reported)
+                else:
+                    warnings.append(reported)
 
             # Find all variable references
             var_names = meta.find_undeclared_variables(ast)

tests/test_template_globals.py

@@ -25,6 +25,7 @@
 import pytest
 
 from orchestrator.core.template_globals import (
+    DEPRECATED,
     GLOBAL_NAMES,
     GLOBAL_SPECS,
     NOT_CALLED,
@@ -40,7 +41,13 @@
 
 
 def _misuse(expression):
-    """The misuse codes a template's expression produces, as a set."""
+    """Codes that would *refuse* the template. Warnings are not refusals."""
+    ast = TemplateManager().env.parse(expression)
+    return {m.code for m in find_global_misuse(ast) if m.severity == "error"}
+
+
+def _reported(expression):
+    """Every code, whatever its severity."""
     ast = TemplateManager().env.parse(expression)
     return {m.code for m in find_global_misuse(ast)}
 
@@ -159,6 +166,26 @@ def test_valid_use_is_left_alone(expression):
     assert _misuse(expression) == set(), f"{expression} is valid and was rejected"
 
 
+def test_a_deprecated_global_is_reported_but_not_refused():
+    """`now()` still runs -- pipelines in the wild use it -- and says so.
+
+    Refusing it would break working pipelines to make a style point. Staying
+    silent would leave authors on a function that gives a different answer to
+    every step of one run.
+    """
+    assert DEPRECATED in _reported("{{ now() }}")
+    assert _misuse("{{ now() }}") == set(), "a deprecated global must still validate"
+
+
+def test_a_global_with_no_replacement_is_not_reported():
+    assert _reported("{{ file_exists('a.txt') }}") == set()
+
+
+def test_deprecation_is_not_reported_for_a_call_that_cannot_work():
+    """One problem at a time: fix the call, then hear about the replacement."""
+    assert _reported("{{ now(1, 2) }}") == {WRONG_ARITY}
+
+
 @pytest.mark.parametrize(
     "expression",
     [
@@ -284,3 +311,30 @@ def test_every_global_appears_in_the_documentation():
     text = (ROOT / "docs" / "template_globals.md").read_text()
     for spec in GLOBAL_SPECS:
         assert f"`{spec.name}`" in text, f"{spec.name} is undocumented"
+
+
+def test_no_example_still_calls_a_deprecated_global():
+    """The catalogue was migrated; this keeps it migrated.
+
+    `{{ now() }}` gave each step of a run a different answer, so the six
+    examples using it stamped their reports inconsistently. They now use
+    `execution.timestamp`.
+    """
+    import re
+
+    deprecated = {s.name for s in GLOBAL_SPECS if s.deprecated_for}
+    if not deprecated:
+        # An empty alternation matches every call, so guard rather than report
+        # the whole catalogue as offending.
+        return
+    pattern = re.compile(r"\{\{[^}]*\b(" + "|".join(sorted(deprecated)) + r")\s*\(")
+
+    offenders = sorted(
+        f"{path.relative_to(ROOT)}: {pattern.search(path.read_text()).group(0)}"
+        for path in (ROOT / "examples").rglob("*.yaml")
+        if pattern.search(path.read_text())
+    )
+    assert not offenders, (
+        "these examples call a deprecated global; the replacement is in "
+        f"docs/template_globals.md: {offenders}"
+    )