Apply suggestions from code review

Co-authored-by: Ada <[email protected]>

Author: sirosen

docs/user_guide/usage_patterns/sessions_and_consents/coalescing_requirements_errors/coalesce_gares.py

@@ -6,35 +6,40 @@ def coalesce(
     session_message: str | None = None,
     prompt: str | None = None,
 ) -> list[globus_sdk.gare.GARE]:
+    # build a list of GARE fields which are allowed to merge
     safe_fields = ["session_required_policies", "required_scopes"]
     if session_message is not None:
         safe_fields.append("session_message")
     if prompt is not None:
         safe_fields.append("prompt")
 
+    # Build lists of GAREs that can and cannot be merged
     candidates, non_candidates = [], []
     for g in gares:
         if _is_candidate(g, safe_fields):
             candidates.append(g)
         else:
             non_candidates.append(g)
 
+    # if no GAREs were safe to merge, return early
     if not candidates:
         return non_candidates
 
+    # merge safe GAREs and override any provided field values
     combined = _safe_combine(candidates)
     if session_message is not None:
         combined.authorization_parameters.session_message = session_message
     if prompt is not None:
         combined.authorization_parameters.prompt = prompt
 
+    # return the reduced list of GAREs
     return [combined] + non_candidates
 
 
 def _is_candidate(g: globus_sdk.gare.GARE, safe_fields: list[str]) -> bool:
     params = g.authorization_parameters
 
-    # look for an "unsafe field" with a value set
+    # check all of the supported GARE fields
     for field_name in (
         "session_message",
         "session_required_identities",
@@ -44,8 +49,11 @@ def _is_candidate(g: globus_sdk.gare.GARE, safe_fields: list[str]) -> bool:
         "required_scopes",
         "prompt",
     ):
+        # if the field is considered safe, ignore it
         if field_name in safe_fields:
             continue
+        # if the field isn't considered safe and it is set,
+        # then the GARE shouldn't be merged
         if getattr(params, field_name) is not None:
             return False
 

docs/user_guide/usage_patterns/sessions_and_consents/coalescing_requirements_errors/index.rst

@@ -48,12 +48,11 @@ These fields are:
 - ``required_scopes``
 - ``session_required_policies``
 
-Additionally, a key field to handle is the ``code``.
+Additionally, the ``code`` field is defined as a non-semantic hint, and is
+therefore safe to rewrite.
 Although services may use other values in practice, there are only two
 well-defined values for the ``code`` string:
 
-There are currently two supported values for the code field:
-
 - ``ConsentRequired``: indicates that the user must consent to additional scopes
   in order to authorize the resource server(s) to complete the requested action.
 - ``AuthorizationRequired``: indicates that this is a Globus Auth Requirements
@@ -62,9 +61,6 @@ There are currently two supported values for the code field:
 Because ``AuthorizationRequired`` is generic, it can always be safely used for
 a ``GARE`` produced from other requirements.
 
-As long as there are no other fields, a pair of ``GARE``\s with these
-safe-to-merge fields can be combined.
-
 Safe Merge Example
 ^^^^^^^^^^^^^^^^^^
 
@@ -235,8 +231,9 @@ applications.
 
 This example merges together these documents, represented as
 :class:`globus_sdk.gare.GARE` objects, only in cases which are defined to
-be safe. If a ``session_message`` or ``prompt`` value is supplied, it will
-be used and will allow the ``GARE``\s to merge more aggressively.
+be safe. If ``session_message`` or ``prompt`` values are supplied, they
+will override any values for these fields present in the GAREs, allowing
+the ``GARE``\s to merge more aggressively.
 
 .. literalinclude:: coalesce_gares.py
     :caption: ``coalesce_gares.py`` [:download:`download <coalesce_gares.py>`]