[feature/capgen-v1]: Test for issue 772 in capgen v0, fix for issue 774 (name collision in capgen v1)

capgen/generator/group_cap.py

@@ -25,7 +25,7 @@
 import os
 from typing import Dict, List, Optional, Set, Tuple
 
-from metadata.parse_tools import FORTRAN_CONDITIONAL_REGEX, open_if_changed
+from metadata.parse_tools import CCPPError, FORTRAN_CONDITIONAL_REGEX, open_if_changed
 from metadata.variable_resolver import HostVarEntry
 from generator.suite_types import _ptr_type_name_for_arg
 from generator.suite_resolver import (
@@ -1187,6 +1187,68 @@ def _generate_state_dealloc(suite_name: str, group_name: str) -> List[str]:
     ]
 
 
+def _check_host_control_local_collisions(
+    suite_name: str,
+    group_name: str,
+    resolved_group: ResolvedGroup,
+    host_dict,
+) -> None:
+    """Reject a host variable whose Fortran local name collides with a control variable.
+
+    In a generated group cap, host variables are use-associated at *module*
+    scope while every control variable is a *subroutine argument*.  When a
+    host variable and a control variable share a Fortran local name, the dummy
+    silently shadows the host import, i.e. is used instead of the host import.
+    Since we cannot rename the variables (they are defined in metadata), an
+    error is thrown when two host variables have the same local name.
+
+    Suite-owned variables cannot collide this way -- they are always prefixed
+    with the suite-data DDT (``ccpp_suite_data(:)%<name>``), and generator
+    locals (transformation temporaries, subcycle counters) are uniquified
+    separately.
+    """
+    if host_dict is None:
+        return
+    control_by_name: Dict[str, HostVarEntry] = {}
+    for entry in _ctrl_entries_for_signature(
+        host_dict, exclude={'suite_name', 'group_name'}
+    ):
+        control_by_name[entry.local_name.lower()] = entry
+    if not control_by_name:
+        return
+
+    def _raise(host_std: str, host_local: str) -> None:
+        ctrl = control_by_name[host_local.lower()]
+        raise CCPPError(
+            "Local name collision in group '{grp}' of suite '{suite}': host "
+            "variable '{hstd}' and control variable '{cstd}' both use the "
+            "Fortran local name '{ln}'. Rename the local_name "
+            "of one of them in the host metadata.".format(
+                grp=group_name, suite=suite_name,
+                hstd=host_std, cstd=ctrl.standard_name, ln=ctrl.local_name,
+            )
+        )
+
+    for items in resolved_group.phase_calls.values():
+        for call in iter_phase_calls(items):
+            for arg in call.args:
+                # Direct host argument (bare use-associated module symbol).
+                if (arg.source == 'host' and arg.module_name is not None
+                        and arg.root_symbol.lower() in control_by_name):
+                    _raise(arg.standard_name, arg.root_symbol)
+                # Dimension-helper and active-condition host variables are
+                # use-associated too, so they can shadow a control dummy.
+                dim_and_active = set(arg.used_dim_std_names)
+                dim_and_active.update(_active_std_names(arg.active))
+                for std in dim_and_active:
+                    entry = host_dict.get(std)
+                    if (entry is not None and not entry.is_control
+                            and entry.module_name is not None):
+                        root = _root_symbol(entry.access_path)
+                        if root.lower() in control_by_name:
+                            _raise(std, root)
+
+
 def _generate_group_cap(
     suite_name: str,
     group_name: str,
@@ -1209,6 +1271,12 @@ def _generate_group_cap(
     -------
     list of str (without trailing newlines)
     """
+    # Fail loudly on a host/control local-name collision before emitting a
+    # cap that would silently read the wrong variable (issue #774).
+    _check_host_control_local_collisions(
+        suite_name, group_name, resolved_group, host_dict
+    )
+
     mod_name = 'ccpp_{}_{}_{}'.format(suite_name, group_name, 'cap')
     # Short Fortran symbols for the state-management subroutines; the
     # module name carries ``ccpp_<suite>_<group>_`` and keeps the mangled

doc/followups.md

@@ -47,6 +47,7 @@ Status values: `open`, `in progress`, `blocked`, `closed`.
 | FU-029 | Decide `timestep_init` / `timestep_final` phase-call-count semantics | framework | 2026-06-10 | open | For a scheme that appears multiple times in a suite, original capgen calls its `timestep_init`/`final` **once per appearance**; capgen-ng calls it **once per group** (measured cam4: `qneg_timestep_final` 2 vs 12).  Benign for cam4 (the affected phases are idempotent/guarded) but a latent b4b/correctness hazard the moment such a phase is stateful (accumulates, zeroes a buffer).  CCPP intent is once-per-timestep; neither matches strictly when a scheme spans groups.  Decide the intended semantics and make capgen-ng's behaviour intentional + documented.  Reproduce via the standalone-capgen driver, diffing `_timestep_(init|final)` call counts. |
 | FU-030 | Deterministic + documented constituent registration order in the generator | framework | 2026-06-11 | open | Root cause of the cam4 FWAUT b4b diff (the framework side of FU-018): capgen-ng registers water species alphabetically ([cloud_ice, cloud_liquid, water_vapor]) vs original's declaration order ([cloud_liquid, cloud_ice, water_vapor]), and trace gases differ too, so `air_composition`'s `thermodynamic_active_species_idx` order → `get_hydrostatic_energy` water-sum FP order → energy fixer → pervasive roundoff.  Proven b4b by a flag-guarded reorder hack.  **Decision (Dom): RE-BASELINE** — give capgen-ng a deterministic, documented order (qv first; an understandable rule for how constituents land in the array), then CAM-SIMA re-baselines against the original-capgen reference; not match-the-old-order.  Levers: `host_constituents.py` / the legacy-auto-clone path (FU-012) / `ccpp_register_constituents` emission; intersects the constituents overhaul (FU-020).  Analysis: `doc/cam4_fwaut_constituent_order.md`. |
 | FU-031 | Long-term redesign of the `ccpp_static_api.F90` runtime listings | framework | 2026-05-14 | open | The suite-variable / suite-host-data listings made the introspection module ~33k lines (`-O3` effectively hangs).  Immediate pressure is off — `--no-host-introspection` stubs them (→ ~800 lines) — so this is **no longer blocking**, but the long-term redesign stays open for team discussion: move the listings to a runtime read of `datatable.xml` (preferred — no recompile when listings change), or a separate `-O0` file, or static string `data` tables, or lazy-emit only the routines the host calls.  Do not redesign unilaterally. |
+| FU-032 | Generator-owned locals can silently shadow a host import — auto-uniquify | framework | 2026-08-07 | open | `_check_host_control_local_collisions` (`capgen/generator/group_cap.py`) now hard-errors when a host variable's local name collides with a control-variable dummy (issue #774 — the silent wrong-value case, closed by that check + `unit-tests/test_suite_resolver.py::TestHostControlLocalNameCollision`).  Two other subroutine-scope locals can shadow a use-associated host import the same way but are **generator-owned**, so the right fix is to rename *them*, not error: transformation temporaries (`<name>_l` / `<name>_p`) and subcycle loop counters (`ccpp_loop_counter*`).  Seed the temp uniquifier (`used_local_names_phase`, `suite_resolver.py:2446`) with the group's host-import symbols + control-dummy names so `_local_name_conflict` renames generator locals away from them.  Rare in practice (suffixed/reserved names) but closes the class.  Deliberately deferred out of the #774 fix (Step 2, 2026-08-07). |
 
 ---
 
@@ -145,6 +146,7 @@ file, per the procedure in the repository's `CLAUDE.md`.
 
 | Machine | Last reconciled | By |
 |---------|-----------------|-----|
+| `dutchman` | 2026-08-07 | folded the issue #772 / #774 session: added FU-032 (generator-local shadow follow-up).  #772 shown to be a non-issue in v1 (cld_shadow e2e reproducer) and #774 detect-and-error landed in `group_cap.py` — both tracked in GitHub, not restated here |
 | `dutchman` | 2026-08-06 | first sweep of this machine; folded its auto-memory investigation notes into new rows FU-025…FU-031, added Codee `use…only:` detail to FU-005, cross-linked FU-018↔FU-030 |
 | `ip-10-0-0-98.ec2.internal` | 2026-07-29 | swept on adding FU-024; local stores unchanged since the previous sweep, nothing new to fold in |
 | `ip-10-0-0-98.ec2.internal` | 2026-07-28 | initial migration — merged `migration.md` §8, `briefing.md` §7.1, `redesign_prompt.md` "Still deferred", plus open items from this machine's auto-memory |

end-to-end-tests/advection/CMakeLists.txt

@@ -5,7 +5,7 @@
 #
 #------------------------------------------------------------------------------
 
-set(SCHEME_FILES "cld_liq" "cld_ice" "apply_constituent_tendencies" "const_indices")
+set(SCHEME_FILES "cld_liq" "cld_ice" "apply_constituent_tendencies" "const_indices" "cld_shadow")
 set(HOST_FILES   "test_host_data" "test_host_mod" "test_host")
 set(SUITE_FILES  "cld_suite.xml")
 set(HOST         "test_host")

end-to-end-tests/advection/README.md

@@ -8,3 +8,8 @@ Contains tests to exercise the capabilities of the constituents object, includin
 - Accessing and modifying a constituent tendency variable
 - Passing around the constituent tendency array
 - Dimensions are case-insensitive
+- Reuse of a constituent or host variable local name by a scheme
+  interstitial with a different standard name (cld_shadow); capgen v1
+  routes each interstitial through the suite-data DDT
+  (`ccpp_suite_data(:)%...`), so the group cap compiles and runs without
+  any local-name collision (see GitHub issues #772 / #774)

end-to-end-tests/advection/cld_shadow.F90

@@ -0,0 +1,41 @@
+! Test parameterization whose local names collide with other identifiers
+! in the group cap: <cld_ice_array> is also the local name of the cloud
+! ice constituent (see cld_ice) and <ncols> is also the local name of the
+! host model horizontal dimension. Both name a scheme-supplied
+! interstitial here, so capgen must rename the group-cap locals.
+!
+
+module cld_shadow
+
+  use ccpp_kinds, only: kind_phys
+
+  implicit none
+  private
+
+  public :: cld_shadow_run
+
+contains
+
+  !> \section arg_table_cld_shadow_run  Argument Table
+  !! \htmlinclude arg_table_cld_shadow_run.html
+  !!
+  subroutine cld_shadow_run(ncol, timestep, cld_ice_array, ncols,       &
+       errmsg, errflg)
+
+    integer,            intent(in)  :: ncol
+    real(kind_phys),    intent(in)  :: timestep
+    real(kind_phys),    intent(out) :: cld_ice_array(:,:)
+    real(kind_phys),    intent(out) :: ncols(:)
+    character(len=512), intent(out) :: errmsg
+    integer,            intent(out) :: errflg
+    !----------------------------------------------------------------
+
+    errmsg = ''
+    errflg = 0
+
+    cld_ice_array(:ncol,:) = timestep
+    ncols(:ncol) = real(ncol, kind_phys)
+
+  end subroutine cld_shadow_run
+
+end module cld_shadow

end-to-end-tests/advection/cld_shadow.meta

@@ -0,0 +1,46 @@
+[ccpp-table-properties]
+  name = cld_shadow
+  type = scheme
+[ccpp-arg-table]
+  name = cld_shadow_run
+  type = scheme
+[ ncol ]
+  standard_name = horizontal_dimension
+  type = integer
+  units = count
+  dimensions = ()
+  intent = in
+[ timestep ]
+  standard_name = time_step_for_physics
+  long_name = time step
+  units = s
+  dimensions = ()
+  type = real | kind = kind_phys
+  intent = in
+[ cld_ice_array ]
+  standard_name = cld_shadow_scratch_array
+  units = s
+  type = real | kind = kind_phys
+  dimensions = (horizontal_dimension, vertical_layer_dimension)
+  intent = out
+[ ncols ]
+  standard_name = cld_shadow_column_scratch
+  units = count
+  type = real | kind = kind_phys
+  dimensions = (horizontal_dimension)
+  intent = out
+[ errmsg ]
+  standard_name = ccpp_error_message
+  long_name = Error message for error handling in CCPP
+  units = none
+  dimensions = ()
+  type = character
+  kind = len=512
+  intent = out
+[ errflg ]
+  standard_name = ccpp_error_code
+  long_name = Error flag for error handling in CCPP
+  units = 1
+  dimensions = ()
+  type = integer
+  intent = out

end-to-end-tests/advection/cld_suite.xml

@@ -7,5 +7,6 @@
     <scheme>apply_constituent_tendencies</scheme>
     <scheme>cld_ice</scheme>
     <scheme>apply_constituent_tendencies</scheme>
+    <scheme>cld_shadow</scheme>
   </group>
 </suite>

unit-tests/test_suite_resolver.py

@@ -5477,6 +5477,110 @@ def test_range_dimension_token_checked(self):
             validate_init_dimensions(sr)
 
 
+########################################################################
+# Host/control local-name collision detection (GitHub issue #774).
+# A host variable and a control variable that share a Fortran local name
+# (with different standard names) must be rejected: in the group cap the
+# host variable is use-associated at module scope while the control
+# variable is a subroutine dummy argument that silently shadows it, so a
+# scheme requesting the host variable would receive the control value.
+########################################################################
+
+_SHADOW_HOST = '''
+[ccpp-table-properties]
+  name = shadow_host
+  type = host
+[ccpp-arg-table]
+  name = shadow_host
+  type = host
+[ nthreads ]
+  standard_name = test_host_thread_count
+  long_name = a host quantity that happens to share the thread-count local name
+  units = count
+  type = integer
+  dimensions = ()
+'''
+
+# Same host table, but the colliding variable is renamed so there is no
+# clash with the control ``nthreads`` (number_of_threads).
+_NOSHADOW_HOST = _SHADOW_HOST.replace('[ nthreads ]', '[ nthr_host ]')
+
+_SHADOW_SCHEME = '''
+[ccpp-table-properties]
+  name = thr_use
+  type = scheme
+[ccpp-arg-table]
+  name = thr_use_run
+  type = scheme
+[ ni ]
+  standard_name = test_host_thread_count
+  units = count
+  type = integer
+  dimensions = ()
+  intent = in
+[ errmsg ]
+  standard_name = ccpp_error_message
+  units = none
+  dimensions = ()
+  type = character
+  kind = len=512
+  intent = out
+[ errflg ]
+  standard_name = ccpp_error_code
+  units = 1
+  dimensions = ()
+  type = integer
+  intent = out
+'''
+
+_SHADOW_SUITE = (
+    '<?xml version="1.0"?>\n'
+    '<suite name="shadow" version="1.0">\n'
+    '  <group name="phys">\n'
+    '    <scheme>thr_use</scheme>\n'
+    '  </group>\n'
+    '</suite>\n'
+)
+
+
+class TestHostControlLocalNameCollision(unittest.TestCase):
+    """A host var and a control var sharing a Fortran local name (different
+    standard names) must be rejected before a cap that silently reads the
+    wrong variable is written (GitHub issue #774).  ``control_full.meta``
+    already declares ``number_of_threads`` with local name ``nthreads``."""
+
+    def _resolve(self, host_src):
+        import logging
+        from generator.suite_xml import parse_suite_xml
+        host_tbls = _parse(host_src)
+        ctrl_tbls = parse_metadata_file(_sf('control_full.meta'))
+        hd = build_flat_host_dict(host_tbls, ctrl_tbls, [])
+        store = SchemeStore.build_from(_parse(_SHADOW_SCHEME))
+        with tempfile.TemporaryDirectory() as tmp:
+            sx = os.path.join(tmp, 'suite_shadow.xml')
+            with open(sx, 'w') as fh:
+                fh.write(_SHADOW_SUITE)
+            suite = parse_suite_xml(sx, tmp, logging.getLogger('test'),
+                                    skip_validation=True)
+            return resolve_suite(suite, store, hd), hd
+
+    def test_collision_raises(self):
+        sr, hd = self._resolve(_SHADOW_HOST)
+        with self.assertRaises(CCPPError) as cm:
+            _generate_group_cap('shadow', 'phys', sr.groups[0], hd)
+        msg = str(cm.exception)
+        self.assertIn('test_host_thread_count', msg)
+        self.assertIn('number_of_threads', msg)
+        self.assertIn('nthreads', msg)
+
+    def test_no_collision_when_local_names_differ(self):
+        # Same host quantity, but its local name no longer matches the
+        # control ``nthreads`` -- generation must succeed.
+        sr, hd = self._resolve(_NOSHADOW_HOST)
+        lines = _generate_group_cap('shadow', 'phys', sr.groups[0], hd)
+        self.assertTrue(any('thr_use_run' in ln for ln in lines))
+
+
 def load_tests(loader, tests, ignore):
     import generator.suite_resolver as suite_resolution
     import generator.group_cap as gc