From 4bf70f6c3030a2b666958b5752f78661b888fc34 Mon Sep 17 00:00:00 2001 From: Marc Verhagen Date: Tue, 7 Jul 2026 12:41:55 -0400 Subject: [PATCH 1/3] Dealing with multi-valued parameters when signing a view. --- clams/app/__init__.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/clams/app/__init__.py b/clams/app/__init__.py index 0519200..7c20a19 100644 --- a/clams/app/__init__.py +++ b/clams/app/__init__.py @@ -311,25 +311,31 @@ def sign_view(self, view: View, runtime_conf: dict) -> None: 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: + self.add_parameters_to_metadata(view, runtime_conf) + + def add_parameters_to_metadata(self, view: View, parameters: dict): + """Add parameters to the view's metadata. Handles parameters that are + defined in the metadata of the app differently from those that are not.""" + # TODO: should this deal differently with parameters that are handed in + # via an envelope? + params_map = {p.name: p for p in self.metadata.parameters} + for k, v in parameters.items(): + if k in params_map: + if params_map[k].multivalued: view.metadata.add_parameter(k, str(v)) else: 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 From 44e8be64175bba2e5341e2d30088c74b6c9bdedd Mon Sep 17 00:00:00 2001 From: Keigh Rim Date: Sun, 12 Jul 2026 14:21:52 -0400 Subject: [PATCH 2/3] updated documentation to clarify `params` and `appConfig` differences --- clams/app/__init__.py | 32 ++++++++++++++++++++++++++----- documentation/app-baseclasses.rst | 5 +++++ documentation/clamsapp.rst | 12 ++++++++++++ documentation/introduction.rst | 2 +- documentation/runtime-params.rst | 10 +++++++++- 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/clams/app/__init__.py b/clams/app/__init__.py index 7c20a19..3758d59 100644 --- a/clams/app/__init__.py +++ b/clams/app/__init__.py @@ -301,12 +301,34 @@ 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]]`` straight from the + HTTP query string or the CLI parser. Only the ``parameters`` field is + populated; there is no resolved configuration to record. + + 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: diff --git a/documentation/app-baseclasses.rst b/documentation/app-baseclasses.rst index 6ac5cfc..6e3b369 100644 --- a/documentation/app-baseclasses.rst +++ b/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 `_ for the definitions of the +``parameters`` and ``appConfiguration`` fields. + What the base class provides ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/documentation/clamsapp.rst b/documentation/clamsapp.rst index b1af2db..76c6c1f 100644 --- a/documentation/clamsapp.rst +++ b/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: diff --git a/documentation/introduction.rst b/documentation/introduction.rst index 96435c5..49ea635 100644 --- a/documentation/introduction.rst +++ b/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 `_. .. 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. diff --git a/documentation/runtime-params.rst b/documentation/runtime-params.rst index 146102e..af9a343 100644 --- a/documentation/runtime-params.rst +++ b/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 `_ for the + authoritative definitions of these two fields. + As HTTP Server -------------- From 289129d42aaef077cd98f1e790e73de0400b4522 Mon Sep 17 00:00:00 2001 From: Keigh Rim Date: Sun, 12 Jul 2026 15:00:33 -0400 Subject: [PATCH 3/3] fixed minor bug in raw parameter recording --- clams/app/__init__.py | 46 +++++++++++++++++++++++++++++------------- tests/test_clamsapp.py | 28 +++++++++++++++++++++++++ tests/test_envelope.py | 13 ++++++++++++ 3 files changed, 73 insertions(+), 14 deletions(-) diff --git a/clams/app/__init__.py b/clams/app/__init__.py index 3758d59..393eac7 100644 --- a/clams/app/__init__.py +++ b/clams/app/__init__.py @@ -313,9 +313,10 @@ def sign_view(self, view: View, runtime_conf: dict) -> None: 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]]`` straight from the - HTTP query string or the CLI parser. Only the ``parameters`` field is - populated; there is no resolved configuration to record. + * "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 @@ -340,21 +341,38 @@ def sign_view(self, view: View, runtime_conf: dict) -> None: else: view.metadata.add_app_configuration(k, v) else: - # meaning the parameters directly from flask or argparser and values are in lists + # 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): - """Add parameters to the view's metadata. Handles parameters that are - defined in the metadata of the app differently from those that are not.""" - # TODO: should this deal differently with parameters that are handed in - # via an envelope? - params_map = {p.name: p for p in self.metadata.parameters} + """ + 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 k in params_map: - if params_map[k].multivalued: - view.metadata.add_parameter(k, str(v)) - else: - view.metadata.add_parameter(k, v[0]) + if len(v) == 0: + continue + elif len(v) == 1: + view.metadata.add_parameter(k, v[0]) else: view.metadata.add_parameter(k, str(v)) diff --git a/tests/test_clamsapp.py b/tests/test_clamsapp.py index 412aa5d..7acac56 100644 --- a/tests/test_clamsapp.py +++ b/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, []) diff --git a/tests/test_envelope.py b/tests/test_envelope.py index b295475..ede75ab 100644 --- a/tests/test_envelope.py +++ b/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):