Warn on malformed variable substitution syntax in dvc.yaml (#177)

Copilot · grst · web-flow · commit 589ba128ea4b · 2026-05-28T16:52:57.000+02:00
* Initial plan

* Add warning for malformed variable substitution syntax in dvc.yaml

When deps/outs entries in dvc.yaml contain '${' but don't have a properly
closed '}', emit a warning to help users identify syntax errors early
rather than silently ignoring the parameter.

Fixes the issue where read_params would not fail or warn on syntax errors
like '${ test_param ]', only causing confusing errors later when the
parameter is accessed.

* Update CHANGELOG.md with entry for malformed variable substitution warning

* Detect additional malformed variable substitution patterns in dvc.yaml

Add detection for common mistakes beyond just missing closing brace:
- $( param ) - wrong brackets (shell syntax confusion)
- $[ param ] - wrong brackets
- ${{ param }} - double braces (GitHub Actions syntax confusion)

Malformed patterns are now checked before the valid pattern to ensure
double-brace cases (which partially match the valid regex) are caught.

---------

Co-authored-by: copilot-swe-agent[bot] &lt;198982749+Copilot@users.noreply.github.com&gt;
Co-authored-by: Gregor Sturm &lt;gregor.sturm@boehringer-ingelheim.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning][].
 
 ### Additions
 
+-  `read_params`/`get_config` now warns when `deps`/`outs` sections in `dvc.yaml` contain malformed variable substitution syntax (e.g. `${ param ]` instead of `${ param }`) ([#151](https://github.com/Boehringer-Ingelheim/dso/issues/151))
 -  Watermarks in PDF and SVG are now natively implemented in these formats as text, rather than including an image overlay ([#170](https://github.com/Boehringer-Ingelheim/dso/pull/170))
 -  JSON output support for `dso get-config` ([#157](https://github.com/Boehringer-Ingelheim/dso/issues/157))
 -  Improve shell autocompletion by adding `click.Path()` type to path arguments in CLI commands ([#173](https://github.com/Boehringer-Ingelheim/dso/pull/173))
diff --git a/src/dso/_get_config.py b/src/dso/_get_config.py
@@ -107,12 +107,26 @@ def get_config(stage: str, *, all: bool = False, skip_compile: bool = False) ->
         else:
             keep_params = set(params)
         dvc_param_pat = re.compile(r"\$\{\s*(.*?)\s*\}")
-        for dep in dvc_stage_config.get("deps", []):
-            if match := dvc_param_pat.findall(dep):
-                keep_params.update(match)
-        for out in dvc_stage_config.get("outs", []):
-            if match := dvc_param_pat.findall(out):
-                keep_params.update(match)
+        # Patterns to detect common malformed variable substitution attempts
+        _malformed_patterns = [
+            # ${{ ... }} double braces (e.g. GitHub Actions syntax confusion)
+            re.compile(r"\$\{\{"),
+            # ${ without matching } (e.g. "${ param ]", "${ param )", "${ param")
+            re.compile(r"\$\{[^}]*$"),
+            # $( ... ) instead of ${ ... }
+            re.compile(r"\$\("),
+            # $[ ... ] instead of ${ ... }
+            re.compile(r"\$\["),
+        ]
+        for section in ("deps", "outs"):
+            for entry in dvc_stage_config.get(section, []) or []:
+                if any(pat.search(entry) for pat in _malformed_patterns):
+                    log.warning(
+                        f"Possible syntax error in `{section}` section of `dvc.yaml`: `{entry}`. "
+                        "Variable substitution uses the syntax `${ <param_name> }`."
+                    )
+                elif match := dvc_param_pat.findall(entry):
+                    keep_params.update(match)
 
         log.info(
             f"Only including the following parameters which are listed in `dvc.yaml`: [green]{', '.join(keep_params)}"
diff --git a/tests/test_get_config.py b/tests/test_get_config.py
@@ -181,6 +181,127 @@ def test_get_config_matrix(dso_project):
     assert list(config) == ["A"]
 
 
+def test_get_config_malformed_variable_substitution(dso_project, capfd):
+    """Test that get_config warns when dvc.yaml contains malformed variable substitution syntax"""
+    chdir(dso_project)
+    stage = dso_project / "mystage"
+    stage.mkdir()
+    (stage / "params.in.yaml").touch()
+
+    (dso_project / "params.in.yaml").write_text(
+        dedent(
+            """\
+            test_param: "output/test.txt"
+            other_param: "something"
+            """
+        )
+    )
+
+    (stage / "dvc.yaml").write_text(
+        dedent(
+            """\
+            stages:
+               mystage01:
+                 params:
+                   - other_param
+                 outs:
+                   - ${ test_param ]
+                 cmd: "echo Hello World!"
+            """
+        )
+    )
+
+    config = get_config("mystage")
+    # The malformed entry should not be parsed as a parameter
+    assert config == {"other_param": "something"}
+    # Check that a warning was emitted
+    captured = capfd.readouterr()
+    assert "Possible syntax error" in captured.err or "Possible syntax error" in captured.out
+
+
+def test_get_config_malformed_variable_substitution_deps(dso_project, capfd):
+    """Test that get_config warns when deps section has malformed variable substitution"""
+    chdir(dso_project)
+    stage = dso_project / "mystage"
+    stage.mkdir()
+    (stage / "params.in.yaml").touch()
+
+    (dso_project / "params.in.yaml").write_text(
+        dedent(
+            """\
+            test_param: "input/test.txt"
+            other_param: "something"
+            """
+        )
+    )
+
+    (stage / "dvc.yaml").write_text(
+        dedent(
+            """\
+            stages:
+               mystage01:
+                 params:
+                   - other_param
+                 deps:
+                   - ${ test_param ]
+                 cmd: "echo Hello World!"
+            """
+        )
+    )
+
+    config = get_config("mystage")
+    assert config == {"other_param": "something"}
+    captured = capfd.readouterr()
+    assert "Possible syntax error" in captured.err or "Possible syntax error" in captured.out
+
+
+@pytest.mark.parametrize(
+    "malformed_entry",
+    [
+        "${ test_param ]",  # ] instead of }
+        "${ test_param )",  # ) instead of }
+        "${ test_param",  # no closing brace at all
+        "$( test_param )",  # wrong brackets $()
+        "$[ test_param ]",  # wrong brackets $[]
+        "${{ test_param }}",  # double braces (GitHub Actions syntax)
+    ],
+)
+def test_get_config_malformed_variable_patterns(dso_project, capfd, malformed_entry):
+    """Test that get_config warns for various common malformed variable substitution patterns"""
+    chdir(dso_project)
+    stage = dso_project / "mystage"
+    stage.mkdir()
+    (stage / "params.in.yaml").touch()
+
+    (dso_project / "params.in.yaml").write_text(
+        dedent(
+            """\
+            test_param: "output/test.txt"
+            other_param: "something"
+            """
+        )
+    )
+
+    (stage / "dvc.yaml").write_text(
+        dedent(
+            f"""\
+            stages:
+               mystage01:
+                 params:
+                   - other_param
+                 outs:
+                   - {malformed_entry}
+                 cmd: "echo Hello World!"
+            """
+        )
+    )
+
+    config = get_config("mystage")
+    assert config == {"other_param": "something"}
+    captured = capfd.readouterr()
+    assert "Possible syntax error" in captured.err or "Possible syntax error" in captured.out
+
+
 def test_get_config_path_relative_to_root_dir(quarto_stage):
     chdir(quarto_stage)
     config1 = get_config("quarto_stage")
