Upgrade ruff to 0.16.4 and ty to 0.0.74 - #103
Draft
davegaeddert wants to merge 2 commits into
Draft
Conversation
ty's stricter checking surfaced a wide class of latent str | None issues around Field.name (only None before contribute_to_class), plus a genuinely non-generic ImmutableList(tuple) subclass and a few descriptor-protocol false positives. Retyping Field.name as str (default "") and making ImmutableList generic removes ~30 scattered `assert x.name is not None` guards instead of adding new ones. Other fixes: SimpleCookie is not itself generic in typeshed (it's already BaseCookie[str]), UnicodeError.reason needs narrowing to its three concrete subclasses, and a couple of list[...] parameters were widened to Sequence[...] where ImmutableList was being passed in. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015HY8ykWPUqsiwvfAJ38oLM
`import plainx` in plain/cli/agent.py carried a `# ty: ignore[unresolved-import]` because plainx-dev, though a workspace member, was never listed as a dev dependency. A clean `uv sync` — which is what CI's lint job runs — never installs it, so the import only resolved in checkouts whose venv had accumulated it from an earlier `uv sync --all-packages`. Add it as a real dev dependency instead of keeping the ignore, matching the existing openapi-spec-validator precedent for deps that ty needs resolvable in every environment. Verified from a deleted .venv, which is the only way to reproduce CI's resolution. Credit to #99, which found this independently. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015HY8ykWPUqsiwvfAJ38oLM
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.
Supersedes #99, which upgraded to ruff 0.16.3 / ty 0.0.72 a week ago and has since fallen behind master. Both PRs independently diagnosed the same three root causes; this one goes to the newer versions, sits on current master, and takes a different (smaller) approach to the main one — see below.
Summary
ruff0.16.2 → 0.16.4 (within the existing<0.17bound) andty0.0.65 → 0.0.74.Field.namewas typedstr | None(onlyNonebeforecontribute_to_class), which had been masked byImmutableList— used throughoutplain-postgres/meta.pyfor field collections — not actually being generic. FixingImmutableListto a properclass ImmutableList[T](tuple[T, ...])unmasked ~30 sites relying onfield.namealways being a string.Field.nameasstr(default"") at the source. A sub-agent traced every read, write, anddeconstruct()consumer of theNonefirst: nothing branches on it, the one behavioral difference was a__repr__is not Nonecheck (now a truthiness check), anddeconstruct()'s name is discarded by every one of its ~15 consumers. This removes the scattered asserts rather than adding an accessor to work around them.plain-postgres's related-field classes:Fieldimplements__get__, so a class-bodyfield: ForeignKeyFieldannotation made ty resolve everyself.fieldread through the descriptor protocol. Moved those annotations to the assignment site in__init__, which matches real Python semantics.SimpleCookie[str]is an invalid subscript —SimpleCookieis alreadyBaseCookie[str], not itself generic.UnicodeError.reasonnarrowed viaisinstanceagainst the three concrete stdlib subclasses that declare it, replacing ahasattrduck-check that typed it asAny.list[Field]parameters that only read/iterate their input toSequence[Field], so anImmutableListpasses without copying.plainx-devinto the dev dependency group. It's a workspace member, butimport plainxinplain/cli/agent.pycarried a# ty: ignore[unresolved-import]because a cleanuv sync— what CI's lint job runs — never installed it; it only resolved in checkouts whose venv had accumulated it from an earlier--all-packagessync. Credit to Upgrade ruff to 0.16.3 and ty to 0.0.72 #99, which found this independently; verified here from a deleted.venv, the only way to reproduce CI's resolution./simplifyon the resulting diff (4 parallel review agents: reuse, simplification, efficiency, altitude) and applied what it found — reverted a redundant cached attribute, relocatedget_joining_columnsfromForeignObjectReldown toForeignKeyRelwhere it's actually reachable (deleting aNotImplementedErrorstub and narrowing twojoin_fieldannotations to match), swapped a semantically-wrongTypeIsforTypeGuard, and replaced adict.update()stdlib bypass with an approach that preservesMorsel's own reserved-key validation.Net effect on the repo's own type-health counters (
./scripts/type-validate), measured against master's source on the new ty:cast()callsassertstatementsTest plan
uv run plain-code check .(ruff + ty + annotations) clean — oxlint/oxfmt skipped, no JS/CSS touched and their binary download is blocked by this sandbox's network policylintsteps reproduced from a deleted.venv:./scripts/type-validate26/26,./scripts/test-package-importsall clean./scripts/test --server) passes across all packages, plus server conformance/load/resilience and MCP conformanceplain-portal's birthday-paradox random-code-collision assertion) reproduced and confirmed pre-existing by rerunning in isolation