fix: clear the remaining type diagnostics and enforce the check - #71
Merged
cigamit merged 1 commit intoSep 13, 2026
Merged
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
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.
blaipr
force-pushed
the
fix/clear-the-remaining-type-diagnostics
branch
from
September 13, 2026 22:50
591c515 to
0e0134b
Compare
cigamit
approved these changes
Sep 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Token_Auth.__call__renamed its parameter,Notification.wait_until_completeddropped**kwargs, andWorkflowJobRelaunch.add_argumentsdroppedwith_pk, which the two otherLaunchableoverrides already carry through.exceptions.Common.msgis the decoded response bodyrather than the empty string its default suggests, the registry metaclass reads a
nameoff classes__subclasses__()describes as itself, andPageListreaches aPageconstructor throughself.__class__.CustomCommand.nameraisedNotImplementedError, but the registryreads it off the class, where subclasses set it as a plain attribute. It is now declared the way
CustomActionalready declaresactionandresource.Connection.request, whose inferred type does not admit the headers added to itfurther down.
parserinsphinx.py, bound from the module__getattr__that PEP 562 routes thefirst lookup to. Annotating it leaves the module namespace empty, so that lookup still arrives there, and
gives the assignment a declaration to point at.
docutilsandsphinxcontrib.autoprogramare thedocsextra, which the job nowinstalls.
simplejson,jqandIPythonare none of them a dependency, so the rule is turned off for thethree files holding them through
[[tool.ty.overrides]].Why an override rather than a
ty: ignoreon the lineThe narrower suppression is the wrong one here. Whether those three resolve depends on the environment the
checker runs in, and once the import does resolve the suppression goes unused, which is itself a diagnostic
and exits non-zero. Making the job enforcing is what turns that into a real cost: CI would go red over a
comment, on a change that never touched this code.
jqis the standing case of the two, since it ships as acompiled extension with no stubs and so fails to resolve whether or not it is installed.
Testing
Every job run locally on the interpreters CI uses, not just on the one to hand:
ruff format --checkandruff checkcleanAll checks passed!sphinx-build -W --keep-goingsucceededpython -m buildandtwine checkPASSED, wheel installs,ascender --helpexits 0The single skip is
tests/unit/cli/test_sphinx.py, which wants thedocsextra the Test job does not install.The type check was also run with
simplejson,IPythonandjqinstalled and then absent, and comes backclean either way, which is the point of the override.