Add USER_CONTENT merge tests + minor review cleanups

Address code-review feedback on the USER_CONTENT merge change:

DEF-010 — Unit tests for _user_code_tuples merge path:
  - Emits an addl USER_CONTENT file not covered by the walker.
  - Drops an addl file whose arcname was already yielded by the walker
    (walker's copy wins).
  - USER_CONTENT bypasses the walker's suffix/user filter, so a file
    excluded by the filter still gets packaged when contributed via
    add_to_package.

DEF-011 — Integration tests covering the two-phase design:
  - _add_addl_files records USER_CONTENT, then _user_code_tuples merges
    with the walker without duplicating an arcname already present in
    the flow dir.
  - A decorator can contribute a file that lives outside the flow dir;
    it flows end-to-end into the user code tuples.

Minor:
  - Remove unused Optional import in package_suffixes_mutator.py.
  - Document why _add_tuple intentionally skips USER_CONTENT (mfcontent
    vs. user-code namespaces are separate).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: romain-intel

metaflow/package/__init__.py

@@ -639,6 +639,11 @@ def _check_tuple(path_tuple):
             return path_tuple
 
         def _add_tuple(path_tuple):
+            # USER_CONTENT is intentionally NOT handled here: those files are
+            # packaged alongside the user's flow code (see _user_code_tuples)
+            # rather than under the mfcontent namespace, and are tracked in
+            # self._user_content_from_addl by _check_tuple above. mfcontent
+            # owns MODULE/CODE/OTHER only.
             file_path, file_name, file_type = path_tuple
             if file_type == ContentType.MODULE_CONTENT:
                 # file_path is actually a module

metaflow/plugins/package_suffixes_mutator.py

@@ -24,7 +24,7 @@ def start(self):
 
 import inspect
 import os
-from typing import List, Optional, Union
+from typing import List, Union
 
 from metaflow.packaging_sys import ContentType
 from metaflow.packaging_sys.utils import walk

test/unit/test_add_to_package.py

@@ -5,8 +5,10 @@
 """
 
 import os
+import sys
 import tempfile
 from types import ModuleType
+from unittest import mock
 from unittest.mock import MagicMock, call
 
 import pytest
@@ -262,3 +264,182 @@ def test_user_content_duplicate_different_path_raises():
         with pytest.raises(NonUniqueFileNameToFilePathMappingException):
             pkg = _build_pkg(flow, _make_environment(), _make_mfcontent())
             pkg._add_addl_files()
+
+
+# ---------------------------------------------------------------------------
+# _user_code_tuples — merge of USER_CONTENT with the flow-dir walker (DEF-010)
+# ---------------------------------------------------------------------------
+
+
+def _build_pkg_for_user_tuples(tmpdir, user_content_from_addl=None):
+    """Build a minimal MetaflowPackage with just enough state for
+    _user_code_tuples(): a flow dir, filter, exclude list, and the
+    dict of USER_CONTENT files produced by add_to_package."""
+    pkg = object.__new__(MetaflowPackage)
+    pkg._user_code_filter = lambda _name: True
+    pkg._exclude_tl_dirs = []
+    pkg._user_content_from_addl = user_content_from_addl or {}
+    pkg._user_flow_dir = None
+    return pkg
+
+
+def _fake_flow_dir(tmpdir, *relpaths):
+    """Create `flow.py` plus the given relative paths in tmpdir. Returns the
+    absolute flow.py path."""
+    flow_file = os.path.join(tmpdir, "flow.py")
+    with open(flow_file, "w") as f:
+        f.write("# flow\n")
+    for rel in relpaths:
+        p = os.path.join(tmpdir, rel)
+        os.makedirs(os.path.dirname(p), exist_ok=True)
+        with open(p, "w") as f:
+            f.write("x\n")
+    return flow_file
+
+
+def test_user_code_tuples_emits_addl_user_content_not_in_walker():
+    """A USER_CONTENT file outside the walker's output gets emitted."""
+    with tempfile.TemporaryDirectory() as flow_dir, tempfile.NamedTemporaryFile(
+        suffix=".cfg", delete=False
+    ) as external:
+        try:
+            _fake_flow_dir(flow_dir, "code.py")
+            pkg = _build_pkg_for_user_tuples(
+                flow_dir,
+                user_content_from_addl={"extra.cfg": external.name},
+            )
+            with mock.patch.object(
+                sys, "argv", [os.path.join(flow_dir, "flow.py")]
+            ), mock.patch("metaflow.R.use_r", return_value=False):
+                tuples = list(pkg._user_code_tuples())
+            by_arc = {arc: path for path, arc in tuples}
+            # walker picked up code.py and flow.py from the flow dir
+            assert "code.py" in by_arc
+            assert "flow.py" in by_arc
+            # external USER_CONTENT was emitted as well
+            assert by_arc["extra.cfg"] == external.name
+        finally:
+            os.unlink(external.name)
+
+
+def test_user_code_tuples_skips_addl_when_walker_already_has_it():
+    """USER_CONTENT with same arcname as a walker-yielded file is dropped."""
+    with tempfile.TemporaryDirectory() as flow_dir:
+        _fake_flow_dir(flow_dir, "code.py")
+        walker_path = os.path.join(flow_dir, "code.py")
+        # A *different* absolute path but same arcname. The walker wins.
+        with tempfile.NamedTemporaryFile(suffix=".py", delete=False) as shadow:
+            try:
+                pkg = _build_pkg_for_user_tuples(
+                    flow_dir,
+                    user_content_from_addl={"code.py": shadow.name},
+                )
+                with mock.patch.object(
+                    sys, "argv", [os.path.join(flow_dir, "flow.py")]
+                ), mock.patch("metaflow.R.use_r", return_value=False):
+                    tuples = list(pkg._user_code_tuples())
+                # code.py appears exactly once, with the walker's path
+                code_py = [t for t in tuples if t[1] == "code.py"]
+                assert len(code_py) == 1
+                assert code_py[0][0] == walker_path
+                # shadow is never emitted
+                assert not any(t[0] == shadow.name for t in tuples)
+            finally:
+                os.unlink(shadow.name)
+
+
+def test_user_code_tuples_respects_user_code_filter():
+    """USER_CONTENT bypasses the suffix/user filter applied to the walker.
+
+    The walker's filter excludes .yaml, but a USER_CONTENT tuple with a .yaml
+    arcname must still be emitted — this is a primary reason USER_CONTENT
+    exists.
+    """
+    with tempfile.TemporaryDirectory() as flow_dir:
+        _fake_flow_dir(flow_dir, "conf.yaml")
+        yaml_path = os.path.join(flow_dir, "conf.yaml")
+        pkg = _build_pkg_for_user_tuples(
+            flow_dir,
+            user_content_from_addl={"conf.yaml": yaml_path},
+        )
+        # Restrict walker to .py only — .yaml should not come from the walker.
+        pkg._user_code_filter = lambda fname: fname.lower().endswith(".py")
+        with mock.patch.object(
+            sys, "argv", [os.path.join(flow_dir, "flow.py")]
+        ), mock.patch("metaflow.R.use_r", return_value=False):
+            tuples = list(pkg._user_code_tuples())
+        by_arc = {arc: path for path, arc in tuples}
+        assert "conf.yaml" in by_arc
+        assert by_arc["conf.yaml"] == yaml_path
+
+
+# ---------------------------------------------------------------------------
+# Integration: _add_addl_files + _user_code_tuples together (DEF-011)
+# ---------------------------------------------------------------------------
+
+
+def test_integration_add_addl_then_user_code_tuples_dedupes_by_arcname():
+    """End-to-end: decorator emits USER_CONTENT for a file already in the
+    flow dir; the final user tuples contain it only once with the walker's
+    path.
+    """
+    with tempfile.TemporaryDirectory() as flow_dir:
+        _fake_flow_dir(flow_dir, "code.py")
+        walker_path = os.path.join(flow_dir, "code.py")
+
+        deco = _make_deco([(walker_path, "code.py", ContentType.USER_CONTENT)])
+        flow = _make_flow(
+            steps=[_make_step()],
+            flow_decorators={"fd": [deco]},
+        )
+        pkg = _build_pkg(flow, _make_environment(), _make_mfcontent())
+        pkg._user_code_filter = lambda _: True
+        pkg._exclude_tl_dirs = []
+        pkg._user_flow_dir = None
+
+        # Phase 1: populate _user_content_from_addl from add_to_package hooks.
+        pkg._add_addl_files()
+        assert pkg._user_content_from_addl == {"code.py": os.path.realpath(walker_path)}
+
+        # Phase 2: walk the flow dir and merge addl USER_CONTENT.
+        with mock.patch.object(
+            sys, "argv", [os.path.join(flow_dir, "flow.py")]
+        ), mock.patch("metaflow.R.use_r", return_value=False):
+            tuples = list(pkg._user_code_tuples())
+
+        # code.py is present exactly once, walker's copy wins (by arcname dedup).
+        code_py = [t for t in tuples if t[1] == "code.py"]
+        assert len(code_py) == 1
+
+
+def test_integration_add_addl_contributes_file_outside_flow_dir():
+    """End-to-end: decorator emits USER_CONTENT for a file that is NOT in the
+    flow dir; it ends up in the user tuples via the merge path.
+    """
+    with tempfile.TemporaryDirectory() as flow_dir, tempfile.NamedTemporaryFile(
+        suffix=".cfg", delete=False
+    ) as external:
+        try:
+            _fake_flow_dir(flow_dir)
+
+            deco = _make_deco(
+                [(external.name, "external.cfg", ContentType.USER_CONTENT)]
+            )
+            flow = _make_flow(
+                steps=[_make_step()],
+                flow_decorators={"fd": [deco]},
+            )
+            pkg = _build_pkg(flow, _make_environment(), _make_mfcontent())
+            pkg._user_code_filter = lambda _: True
+            pkg._exclude_tl_dirs = []
+            pkg._user_flow_dir = None
+
+            pkg._add_addl_files()
+            with mock.patch.object(
+                sys, "argv", [os.path.join(flow_dir, "flow.py")]
+            ), mock.patch("metaflow.R.use_r", return_value=False):
+                tuples = list(pkg._user_code_tuples())
+            by_arc = {arc: path for path, arc in tuples}
+            assert by_arc["external.cfg"] == os.path.realpath(external.name)
+        finally:
+            os.unlink(external.name)