bug and documentation fixes for parameters vs. appConfiguration in view metadata

clams/app/__init__.py

@@ -301,35 +301,81 @@ def sign_view(self, view: View, runtime_conf: dict) -> None:
         """
         A method to "sign" a new view that this app creates at the beginning of annotation.
         Signing will populate the view metadata with information and configuration of this app.
-        The parameters passed to the :meth:`~clams.app.ClamsApp._annotate` must be
-        passed to this method. This means all parameters for "common" configuration that
-        are consumed in :meth:`~clams.app.ClamsApp.annotate` should not be recorded in the
-        view metadata.
+
+        The ``runtime_conf`` argument can be either "refined" or "raw" parameters,
+        and what ends up recorded in the view metadata differs accordingly:
+
+        * "refined" params: the output of :meth:`~clams.app.ClamsApp._refine_params`,
+          identifiable by the presence of the internal ``_RAW_PARAMS_KEY`` marker.
+          These carry both the user's raw input and the fully resolved configuration
+          (declared parameters with their defaults filled in). Signing then populates
+          **two** metadata fields: ``parameters`` with the raw user input (before
+          defaults), and ``appConfiguration`` with the resolved configuration,
+          including the universal parameters (e.g. ``pretty``, ``runningTime``,
+          ``hwFetch``, ``tfSamplingMode``).
+        * "raw" params: an unrefined ``Dict[str, List[str]]``. Only the
+          ``parameters`` field is populated; there is no resolved configuration
+          to record. See :meth:`~clams.app.ClamsApp.add_parameters_to_metadata`
+          for the input channels these values come from and how each is recorded.
+
+        These two cases exist because of two distinct execution paths. On the normal
+        annotation path, :meth:`~clams.app.ClamsApp.annotate` refines the parameters
+        before invoking :meth:`~clams.app.ClamsApp._annotate`, so a well-behaved app
+        forwards those already-refined params here and gets the full two-field record.
+        On the error path (:meth:`~clams.app.ClamsApp.set_error_view`), refinement may
+        be exactly what failed (:meth:`~clams.app.ClamsApp._refine_params` raises on a
+        missing required parameter or an invalid choice) so the error view is signed
+        with the raw params to guarantee the error is still recorded.
+
         :param view: a view to sign
-        :param runtime_conf: runtime configuration of the app as k-v pairs
+        :param runtime_conf: runtime configuration of the app, either refined
+            (output of :meth:`~clams.app.ClamsApp._refine_params`) or raw k-v pairs
         """
         view.metadata.app = str(self.metadata.identifier)
         if self.metadata.app_tags:
             view.metadata.set_additional_property('appTags', list(self.metadata.app_tags))
-        params_map = {p.name: p for p in self.metadata.parameters}
         if self._RAW_PARAMS_KEY in runtime_conf:
             for k, v in runtime_conf.items():
                 if k == self._RAW_PARAMS_KEY:
-                    for orik, oriv in v.items():
-                        if orik in params_map and params_map[orik].multivalued:
-                            view.metadata.add_parameter(orik, str(oriv))
-                        else:
-                            view.metadata.add_parameter(orik, oriv[0])
+                    self.add_parameters_to_metadata(view, v)
                 else:
                     view.metadata.add_app_configuration(k, v)
         else:
-            # meaning the parameters directly from flask or argparser and values are in lists
-            for k, v in runtime_conf.items():
-                if k in params_map and params_map[k].multivalued:
-                    view.metadata.add_parameter(k, str(v))
-                else:
-                    view.metadata.add_parameter(k, v[0])
-
+            # unrefined params passed directly (no _RAW_PARAMS_KEY marker), e.g.
+            # the error path; values are lists
+            self.add_parameters_to_metadata(view, runtime_conf)
+
+    def add_parameters_to_metadata(self, view: View, parameters: dict):
+        """
+        Record raw runtime parameters into the view's ``parameters`` metadata.
+
+        The ``parameters`` argument reaches an app through one of three channels:
+
+        #. a query string, when the app runs as an HTTP server (``app.py``, via
+           :class:`~clams.restify.Restifier`);
+        #. shell arguments, when the app runs as a CLI program (``cli.py``);
+        #. a JSON envelope built with the ``clams envelop`` command (see
+           :mod:`clams.envelop`), whose values are flattened by
+           :func:`~clams.envelop.normalize_params`.
+
+        In every case the values arrive as ``List[str]`` (the shape
+        :meth:`~clams.app.ClamsApp._refine_params` consumes). A single value is
+        recorded bare with the ``List`` wrapper stripped, multiple values as the
+        serialized list, and an empty list skipped as unset (only an envelope
+        such as ``{"x": []}`` produces one, guarding against an ``IndexError``
+        on ``v[0]``).
+
+        :param view: the view being signed
+        :param parameters: raw runtime parameters as ``Dict[str, List[str]]``
+        """
+        for k, v in parameters.items():
+            if len(v) == 0:
+                continue
+            elif len(v) == 1:
+                view.metadata.add_parameter(k, v[0])
+            else:
+                view.metadata.add_parameter(k, str(v))
+
     def set_error_view(self, mmif: Union[str, dict, Mmif], **runtime_conf: List[str]) -> Mmif:
         """
         A method to record an error instead of annotation results in the view

documentation/app-baseclasses.rst

@@ -493,6 +493,11 @@ A consumer of the output MMIF can read the resolved revision directly
 from the view metadata, with no cross-reference to the app metadata
 required.
 
+This ``model`` handling is a specific instance of the general MMIF convention
+for recording runtime configuration; see the view-metadata section of the
+`MMIF specification <https://mmif.clams.ai/1.2.0/>`_ for the definitions of the
+``parameters`` and ``appConfiguration`` fields.
+
 What the base class provides
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

documentation/clamsapp.rst

@@ -299,6 +299,18 @@ The envelope looks like this:
 The output is always raw MMIF, regardless of input format, so downstream
 pipeline steps are unaffected.
 
+.. note::
+
+   The values in the envelope's ``parameters`` block (not to be confused with
+   ``view.metadata.parameters``, the recorded output field) are interpreted as
+   native JSON. Use real numbers, booleans, arrays, and objects (e.g. ``0.7``, ``true``,
+   ``["a", "b"]``, ``{"B": "bars"}``), not the URL-encoded string form. To
+   reproduce a past run from an existing MMIF, copy the target view's
+   ``appConfiguration`` into the envelope's ``parameters``; it holds the fully
+   resolved, native-JSON configuration. Copying from ``view.metadata.parameters``
+   is not reliable, since that field is the raw "as typed" transcript and is not
+   guaranteed to round-trip.
+
 You can still combine the envelope with query string parameters.
 When the same parameter appears in both, the **query string takes priority**,
 which allows quick overrides without editing the parameter file:

documentation/introduction.rst

@@ -69,7 +69,7 @@ When you inherit :class:`~clams.app.ClamsApp`, you need to implement
 As a developer you can expose different behaviors of the ``annotate()`` method by providing configurable parameters as keyword arguments of the method. For example, you can have the user specify a re-sample rate of an audio file to be analyzed by providing a ``resample_rate`` parameter. 
 
 .. note::
-  These runtime configurations are not part of the MMIF input, but for reproducible analysis, you should record these configurations in the output MMIF. 
+  These runtime configurations are not part of the MMIF input. The SDK records them automatically in the output view metadata for reproducibility: the raw user input in ``view.metadata.parameters`` and the fully resolved configuration in ``view.metadata.appConfiguration``. Both fields are defined in the view-metadata section of the `MMIF specification <https://mmif.clams.ai/1.2.0/>`_.
 
 .. note::
   Some runtime parameters are managed by the SDK itself rather than declared per-app. The *universal* parameters in :const:`clams.app.ClamsApp.universal_parameters` are one such set; they are auto-added to every CLAMS app. Specialized base classes (see below) add their own SDK-managed parameter sets on top.

documentation/runtime-params.rst

@@ -32,9 +32,17 @@ annotations. See an example below.
 
       ...
 
-When you use a configuration parameter in your app, you should also expose it 
+When you use a configuration parameter in your app, you should also expose it
 to the user via the app metadata. See :ref:`appmetadata` section for more details.
 
+.. note::
+  Whichever channel supplies them, the SDK records the runtime parameters in the
+  output view metadata: the raw user input (as strings) in
+  ``view.metadata.parameters``, and the fully refined configuration (defaults filled
+  and values cast) in ``view.metadata.appConfiguration``. See the view-metadata
+  section of the `MMIF specification <https://mmif.clams.ai/1.2.0/>`_ for the
+  authoritative definitions of these two fields.
+
 As HTTP Server
 --------------
 

tests/test_clamsapp.py

@@ -262,6 +262,34 @@ def test_sign_view(self):
         self.assertEqual(len(v4.metadata.appConfiguration), 6)
         self.assertEqual(len(v4.metadata.parameters['multivalued_param']), len(str(multiple_values)))
 
+    def test_add_parameters_to_metadata_len_branch(self):
+        # recording branches purely on the number of values, uniformly for
+        # declared and undeclared params (issue #296)
+        m = Mmif(self.in_mmif)
+
+        # single value -> bare value, no list brackets
+        v = m.new_view()
+        self.app.add_parameters_to_metadata(v, {'undeclared': ['solo']})
+        self.assertEqual(v.metadata.parameters['undeclared'], 'solo')
+
+        # multiple values -> serialized list, no data loss
+        v = m.new_view()
+        self.app.add_parameters_to_metadata(v, {'undeclared_multi': ['a', 'b']})
+        self.assertEqual(v.metadata.parameters['undeclared_multi'], str(['a', 'b']))
+
+        # empty list (only an envelope like {"x": []}/{"x": {}} can produce it)
+        # -> treated as unset, skipped, and must not raise IndexError
+        v = m.new_view()
+        self.app.add_parameters_to_metadata(v, {'empty': []})
+        self.assertNotIn('empty', v.metadata.parameters)
+
+        # a declared multivalued param given a single value is also recorded clean
+        self.app.metadata.add_parameter(
+            'mv', 'multivalued', type='string', multivalued=True, default=[])
+        v = m.new_view()
+        self.app.add_parameters_to_metadata(v, {'mv': ['only']})
+        self.assertEqual(v.metadata.parameters['mv'], 'only')
+
     def test_app_tags_default_empty(self):
         # apps that don't declare tags should not write an appTags field to view metadata
         self.assertEqual(self.app.metadata.app_tags, [])

tests/test_envelope.py

@@ -236,6 +236,19 @@ def test_put_envelope(self):
         self.assertEqual(res.status_code, 200)
         Mmif(res.get_data(as_text=True))
 
+    def test_empty_collection_param_does_not_crash(self):
+        # {"x": []} / {"x": {}} normalize to an empty list; recording must skip
+        # the param instead of raising IndexError on v[0] (issue #296)
+        for empty in ([], {}):
+            envelope_str = create_envelope(
+                self.mmif_str, {'emptyparam': empty}
+            )
+            res = self.client.post('/', data=envelope_str)
+            self.assertEqual(res.status_code, 200)
+            out = Mmif(res.get_data(as_text=True))
+            for view in out.views:
+                self.assertNotIn('emptyparam', view.metadata.parameters)
+
     PREFIX = "Invalid input data. See below for validation error."
 
     def test_envelope_missing_mmif(self):