From 0e0134b0c6a32e29fc6ebf8c34a10cf779334bcc Mon Sep 17 00:00:00 2001
From: Blai Peidro <blaipr@hotmail.com>
Date: Mon, 14 Sep 2026 00:42:59 +0200
Subject: [PATCH] fix: clear the remaining type diagnostics and enforce the
 check

The type check job was advisory while the diagnostics already in the tree were
worked through. The last 23 are gone, so it now fails the merge like the other
checks rather than reporting into a job nobody has to read.

What the diagnostics were, and what each one asked for:

- Overrides that narrowed their base signature. Token_Auth.__call__ renamed its
  parameter, Notification.wait_until_completed dropped **kwargs, and
  WorkflowJobRelaunch.add_arguments dropped with_pk, which the two other
  Launchable overrides already carry through.
- Contracts a mixin relies on but does not declare. exceptions.Common.msg is the
  decoded response body rather than the empty string its default suggests, the
  registry metaclass reads a name off classes __subclasses__() describes as
  itself, and PageList reaches a Page constructor through self.__class__.
- CustomCommand.name raised from a property that nothing ever reaches, since the
  registry reads it off the class where subclasses set it as an attribute. It is
  declared the way CustomAction already declares action and resource.
- The request kwargs in Connection.request, whose inferred type does not admit
  the headers added to it further down.
- sphinx.py binds its parser global from the module __getattr__ that PEP 562
  routes the first lookup to. Annotating it leaves the namespace empty, so the
  lookup still arrives there, and gives the assignment a declaration.
- Optional imports. docutils and sphinxcontrib.autoprogram are the docs extra,
  which the job now installs. simplejson, jq and IPython are none of them a
  dependency, so the rule is turned off for the three files holding them through
  [[tool.ty.overrides]]. A `ty: ignore` on the line would have been narrower and
  wrong: it goes unused, and an unused suppression is itself a diagnostic, the
  moment anything pulls one of the three into the environment. Turning the job
  enforcing is what makes that worth avoiding, since it would fail the merge
  over a comment rather than over the code.
---
 .github/workflows/ci.yml               | 10 +++++-----
 ascenderkit/api/client.py              | 14 ++++++++++----
 ascenderkit/api/mixins/has_create.py   |  2 ++
 ascenderkit/api/pages/notifications.py |  4 ++--
 ascenderkit/api/pages/page.py          | 10 ++++++++--
 ascenderkit/cli/custom.py              |  4 ++--
 ascenderkit/cli/format.py              |  2 ++
 ascenderkit/cli/resource.py            |  7 ++++---
 ascenderkit/cli/sphinx.py              |  6 ++++++
 ascenderkit/cli/utils.py               |  6 ++++++
 ascenderkit/exceptions.py              |  8 ++++++++
 ascenderkit/scripts/basic_session.py   |  2 ++
 pyproject.toml                         | 18 ++++++++++++++++++
 tox.ini                                | 11 ++++++-----
 14 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 5306a34..129ebe9 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -81,9 +81,6 @@ jobs:
   type-check:
     name: Type check
     runs-on: ubuntu-latest
-    # Advisory while the diagnostics already in the tree are worked through: the
-    # job reports them, it does not fail the pull request.
-    continue-on-error: true
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
 
@@ -93,9 +90,12 @@ jobs:
 
       - name: Install the package and the type checker
         # ty resolves third-party imports out of the environment the package is
-        # installed in, so the extras have to be here too.
+        # installed in, so the extras have to be here too. docs is among them
+        # because ascenderkit/cli/sphinx.py imports docutils and autoprogram at
+        # module level. formatting is not: jq ships as a compiled extension with
+        # no stubs, so installing it resolves nothing.
         run: |
-          pip install -e ".[websockets,crypto]"
+          pip install -e ".[websockets,crypto,docs]"
           pip install ty==0.0.74
 
       - name: Run the type checker
diff --git a/ascenderkit/api/client.py b/ascenderkit/api/client.py
index 8d20d97..3c62399 100644
--- a/ascenderkit/api/client.py
+++ b/ascenderkit/api/client.py
@@ -1,4 +1,5 @@
 import logging
+from typing import Any
 
 import requests
 from requests.auth import AuthBase
@@ -18,9 +19,11 @@ class Token_Auth(AuthBase):
     def __init__(self, token):
         self.token = token
 
-    def __call__(self, request):
-        request.headers['Authorization'] = f'Bearer {self.token}'
-        return request
+    # Named `r` because AuthBase.__call__ names it that, and a caller holding
+    # the base type is entitled to pass it by that keyword.
+    def __call__(self, r):
+        r.headers['Authorization'] = f'Bearer {self.token}'
+        return r
 
 
 def log_elapsed(r, *args, **kwargs):  # requests hook to display API elapsed time
@@ -87,7 +90,10 @@ def request(self, relative_endpoint, method='get', json=None, data=None, query_p
             use_endpoint = use_endpoint[1:]
         url = '/'.join([self.server, use_endpoint])
 
-        kwargs = dict(verify=self.verify, params=query_parameters, json=json, data=data, hooks=dict(response=log_elapsed))
+        # Annotated because the values are heterogeneous: inference narrows the
+        # dict to the union of the literals below, and the headers added further
+        # down are not in it.
+        kwargs: dict[str, Any] = dict(verify=self.verify, params=query_parameters, json=json, data=data, hooks=dict(response=log_elapsed))
 
         if headers is not None:
             kwargs['headers'] = headers
diff --git a/ascenderkit/api/mixins/has_create.py b/ascenderkit/api/mixins/has_create.py
index 414f039..d7172e3 100644
--- a/ascenderkit/api/mixins/has_create.py
+++ b/ascenderkit/api/mixins/has_create.py
@@ -195,6 +195,8 @@ def _filter_ds_from_payload(obj, *a, **kw):
 json.dumps = filter_ds_from_payload(json.dumps)
 
 try:
+    # Not a dependency: it is patched only when something else in the
+    # environment has brought it in. See [[tool.ty.overrides]] in pyproject.toml.
     import simplejson  # noqa
 
     simplejson.dumps = filter_ds_from_payload(simplejson.dumps)
diff --git a/ascenderkit/api/pages/notifications.py b/ascenderkit/api/pages/notifications.py
index 359c802..83fb63f 100644
--- a/ascenderkit/api/pages/notifications.py
+++ b/ascenderkit/api/pages/notifications.py
@@ -26,12 +26,12 @@ def wait_until_status(self, status, interval=5, timeout=30, **kwargs):
         adjusted_timeout = timeout - seconds_since_date_string(self.created)
         return super().wait_until_status(status, interval, adjusted_timeout, **kwargs)
 
-    def wait_until_completed(self, interval=5, timeout=240):
+    def wait_until_completed(self, interval=5, timeout=240, **kwargs):
         """Notifications need a longer timeout, since the backend often has
         to wait for the request (sending the notification) to timeout itself
         """
         adjusted_timeout = timeout - seconds_since_date_string(self.created)
-        return super().wait_until_completed(interval, adjusted_timeout)
+        return super().wait_until_completed(interval, adjusted_timeout, **kwargs)
 
 
 page.register_page(resources.notification, Notification)
diff --git a/ascenderkit/api/pages/page.py b/ascenderkit/api/pages/page.py
index 38eca35..3aa32d8 100644
--- a/ascenderkit/api/pages/page.py
+++ b/ascenderkit/api/pages/page.py
@@ -344,6 +344,10 @@ class PageList:
     # Page.__init__, next and previous come from the response body through
     # Page.__getattr__, and get is Page's own. Annotations rather than
     # assignments, so nothing is created at runtime.
+    #
+    # self.__class__ is a Page subclass for the same reason. The two methods
+    # below hold it in a local first, because the mixin does not inherit Page
+    # and the only constructor visible behind the class itself is object's.
     json: dict
     connection: Any
     r: Any
@@ -379,12 +383,14 @@ def results(self):
 
     def go_to_next(self):
         if self.next:
-            next_page = self.__class__(self.connection, endpoint=self.next)
+            page_class: Any = self.__class__
+            next_page = page_class(self.connection, endpoint=self.next)
             return next_page.get()
 
     def go_to_previous(self):
         if self.previous:
-            prev_page = self.__class__(self.connection, endpoint=self.previous)
+            page_class: Any = self.__class__
+            prev_page = page_class(self.connection, endpoint=self.previous)
             return prev_page.get()
 
     def create(self, *a, **kw):
diff --git a/ascenderkit/cli/custom.py b/ascenderkit/cli/custom.py
index 75c3224..99b9f77 100644
--- a/ascenderkit/cli/custom.py
+++ b/ascenderkit/cli/custom.py
@@ -257,10 +257,10 @@ class AdHocCommandRelaunch(HasRelaunch, CustomAction):
 class WorkflowJobRelaunch(HasRelaunch, CustomAction):
     resource = 'workflow_jobs'
 
-    def add_arguments(self, parser, resource_options_parser):
+    def add_arguments(self, parser, resource_options_parser, with_pk=True):
         # The endpoint takes an empty serializer, so OPTIONS advertises no
         # fields and nothing below would be generated from it.
-        super().add_arguments(parser, resource_options_parser)
+        super().add_arguments(parser, resource_options_parser, with_pk=with_pk)
         parser.choices[self.action].add_argument(
             '--nodes',
             choices=['all', 'failed'],
diff --git a/ascenderkit/cli/format.py b/ascenderkit/cli/format.py
index 1c98c6a..e5c08c8 100644
--- a/ascenderkit/cli/format.py
+++ b/ascenderkit/cli/format.py
@@ -148,6 +148,8 @@ def format_response(response, fmt='json', filter='.', changed=False):
 
 def format_jq(output, fmt):
     try:
+        # Ships as a compiled extension with no stubs, so it does not resolve to
+        # a type checker even when installed. See pyproject.toml.
         import jq
     except ImportError:
         if fmt == '.':
diff --git a/ascenderkit/cli/resource.py b/ascenderkit/cli/resource.py
index 3038bc2..bc95e04 100644
--- a/ascenderkit/cli/resource.py
+++ b/ascenderkit/cli/resource.py
@@ -54,9 +54,10 @@ class CustomCommand(metaclass=CustomRegistryMeta):
 
     help_text = ''
 
-    @property
-    def name(self):
-        raise NotImplementedError()
+    # Set as a plain class attribute by every subclass, so it is declared here
+    # rather than raised from a property that nothing ever reaches: the registry
+    # reads it off the class, where a property is the property itself.
+    name: str
 
     def handle(self, client, parser):
         """To be implemented by subclasses.
diff --git a/ascenderkit/cli/sphinx.py b/ascenderkit/cli/sphinx.py
index 78ee507..0cdd19e 100644
--- a/ascenderkit/cli/sphinx.py
+++ b/ascenderkit/cli/sphinx.py
@@ -5,6 +5,12 @@
 
 from .client import CLI
 from .resource import is_control_resource, CustomCommand
+from .utils import HelpfulArgumentParser
+
+# Bound by __getattr__ below on the first lookup, and by nothing else. An
+# annotation rather than an assignment, so the name stays out of the module
+# namespace and PEP 562 still routes that first lookup here.
+parser: HelpfulArgumentParser
 
 
 class CustomAutoprogramDirective(AutoprogramDirective):
diff --git a/ascenderkit/cli/utils.py b/ascenderkit/cli/utils.py
index b97d435..454257b 100644
--- a/ascenderkit/cli/utils.py
+++ b/ascenderkit/cli/utils.py
@@ -24,6 +24,12 @@
 
 
 class CustomRegistryMeta(type):
+    # Every registered class carries one, either as a plain attribute or as a
+    # property on the metaclass that derives it. Declared here because the
+    # registry below reaches those classes through __subclasses__(), which
+    # describes them as this class rather than as themselves.
+    name: str
+
     @property
     def registry(cls):
         return dict((command.name, command) for command in cls.__subclasses__())
diff --git a/ascenderkit/exceptions.py b/ascenderkit/exceptions.py
index 2f7a8fb..c33419b 100644
--- a/ascenderkit/exceptions.py
+++ b/ascenderkit/exceptions.py
@@ -1,4 +1,12 @@
+from typing import Any
+
+
 class Common(Exception):
+    # The decoded response body, which the platform renders as a dict for most
+    # errors, and as a string for the ones it does not serialize. Callers read
+    # it both ways, so it is not narrowed to either.
+    msg: Any
+
     def __init__(self, status_string='', message=''):
         if isinstance(status_string, Exception):
             self.status_string = ''
diff --git a/ascenderkit/scripts/basic_session.py b/ascenderkit/scripts/basic_session.py
index 0158fa2..8125de1 100755
--- a/ascenderkit/scripts/basic_session.py
+++ b/ascenderkit/scripts/basic_session.py
@@ -107,6 +107,8 @@ def load_interactive():
         return main()
 
     try:
+        # Not a dependency: the session falls back to code.interact without it.
+        # See [[tool.ty.overrides]] in pyproject.toml.
         from IPython import start_ipython
 
         basic_session_path = os.path.abspath(__file__)
diff --git a/pyproject.toml b/pyproject.toml
index 0e6005b..a09fa58 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -105,6 +105,24 @@ select = [
     "W291", "W605",
 ]
 
+[[tool.ty.overrides]]
+# Three optional imports the package does not declare: simplejson is patched
+# only when something else in the environment has brought it in, IPython decides
+# which interactive session ascender-shell drops into, and jq backs `-f jq`.
+# Whether each one resolves depends on the environment the checker runs in, so
+# the rule is turned off for the files holding them. A `ty: ignore` on the line
+# would not do: it becomes an unused suppression, and a warning, the moment one
+# of the three is installed. jq is the permanent case of the two, since it ships
+# as a compiled extension with no stubs and so resolves either way.
+include = [
+    "ascenderkit/api/mixins/has_create.py",
+    "ascenderkit/cli/format.py",
+    "ascenderkit/scripts/basic_session.py",
+]
+
+[tool.ty.overrides.rules]
+unresolved-import = "ignore"
+
 [tool.pytest.ini_options]
 # Moved here from a [pytest] section in tox.ini. pytest reads both, and this is
 # the file the rest of the tooling configuration already lives in.
diff --git a/tox.ini b/tox.ini
index fb63648..73d82f2 100644
--- a/tox.ini
+++ b/tox.ini
@@ -44,13 +44,14 @@ commands =
     sphinx-build -b html -W --keep-going ascenderkit/cli/docs/source {envtmpdir}/html
 
 [testenv:typecheck]
-# Advisory: the type checker runs over ascenderkit and reports what it finds
-# without failing the merge, so the diagnostics already in the tree can be
-# worked through a module at a time. It resolves third-party imports out of the
-# environment the package is installed in; set TYPECHECK_PYTHON to point it at
-# one. Pinned like the other linters so results do not drift.
+# The type checker runs over ascenderkit and has to come back clean. It resolves
+# third-party imports out of the environment the package is installed in; set
+# TYPECHECK_PYTHON to point it at one. That is also why docs is installed here,
+# since ascenderkit/cli/sphinx.py imports docutils and autoprogram at module
+# level. Pinned like the other linters so results do not drift.
 deps =
     {[testenv]deps}
+    .[docs]
     ty==0.0.74
 passenv =
     TYPECHECK_PYTHON