Adding Copilot review: Improve load_all handling and CLI logout parser

Enforce and clarify pagination/parameter behavior and fix CLI parser inheritance.

- brainstem_api_client: always pass options to _build_url; disallow load_all=True when an id is provided; raise a clear ValueError if auto-pagination expects a list but the API response contains no list-valued key; change filter merging to include falsy values (check for None instead of truthiness).
- cli: make the logout subparser inherit common arguments (parents=[common]).
- tests: set mocked response.status_code to 200 for pagination tests.
- README: remove the Contributing section.

These changes make load_all usage safer and errors clearer, ensure logout CLI gets shared options, and update tests to better mock HTTP responses.

Author: petersenpeter

README.md

@@ -103,9 +103,6 @@ client.save('session', id='<session-uuid>', data={'description': 'updated'})
 client.delete('session', id='<session-uuid>')
 ```
 
-## Contributing
-Contributions are welcome! Feel free to open issues or submit pull requests on GitHub.
-
 ## Command-line Interface
 
 After installation a `brainstem` command is available in your shell.

brainstem_api_tools/brainstem_api_client.py

@@ -270,7 +270,7 @@ def load(self,
         model = _resolve_model(model)
         portal = _resolve_portal(portal)
         app = _MODEL_TO_APP[model]
-        url = self._build_url(portal, app, model, id, options if id else None)
+        url = self._build_url(portal, app, model, id, options)
 
         params = {}
         if not id:
@@ -288,6 +288,9 @@ def load(self,
         if not load_all:
             return self._session.get(url, params=params, timeout=self.DEFAULT_TIMEOUT)
 
+        if id is not None:
+            raise ValueError("load_all=True cannot be used together with id.")
+
         # --- auto-paginate and merge all pages ---
         page_size = limit or 100
         params["limit"] = page_size
@@ -310,6 +313,11 @@ def load(self,
                 records_key = next(
                     (k for k, v in data.items() if isinstance(v, list)), None
                 )
+                if records_key is None:
+                    raise ValueError(
+                        "load_all=True requires a paginated list response, but the API "
+                        "returned no list-valued key. Use load_all=False for single-object endpoints."
+                    )
                 combined = {k: v for k, v in data.items() if k != records_key}
                 combined[records_key] = []
 
@@ -396,7 +404,7 @@ def _convenience_load(self, model, default_include, filter_map,
         merged_filters = dict(filters or {})
         for kwarg, api_field in filter_map.items():
             value = field_kwargs.get(kwarg)
-            if value:
+            if value is not None:
                 merged_filters[api_field] = value
         return self.load(
             model,

brainstem_api_tools/cli.py

@@ -60,6 +60,7 @@ def _build_parser() -> argparse.ArgumentParser:
     # ---- logout -----------------------------------------------------
     sub.add_parser(
         "logout",
+        parents=[common],
         help="Remove the cached API token.",
     )
 

tests/test_client.py

@@ -303,6 +303,7 @@ def _mock_pages(self, pages):
         responses = []
         for page in pages:
             r = MagicMock()
+            r.status_code = 200
             r.raise_for_status = MagicMock()
             r.json.return_value = page
             responses.append(r)