Repository navigation
ci: fix the no-first-party-patch failures in the Python hermetic tier - #1026
Merged
thekevinscott merged 4 commits intoSep 4, 2026
Merged
Conversation
`asyncio.to_thread` and `os.getcwd` were reached through `dirsql._async`, so upstream's patch.object resolution reads them as first-party targets. Both modules are already imported in the test files; patching them at their own home is the same object with the same scope. skip-changelog: test-only change, no published artifact affected
Patching `dirsql._async._RustDirSQL` is the hermetic tier's definition -- it is what lets that subdir run with no native build and no real SQLite. The rule redirects a first-party patch to a third-party target, and there is none here: `dirsql._dirsql` is first-party too, and rebinding it after `_async` has bound the name would not take effect. The rule does not describe this seam, so the two suites take a scoped waiver. The repo's exemption count moves from zero to one. AGENTS.md and the testing-gates reference said zero and stays there; they now say what the one entry is and why the rule does not apply to it. skip-changelog: gate config and docs, no published artifact affected
skip-changelog: docs only
This was referenced Sep 2, 2026
… claude/tackle-1024-nfpp
8 tasks done
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.
Fixes #1024
integration lint'sno-first-party-patchrule now resolves thepatch.object(...)object form, which made five previously-invisible patches inpackages/python/tests/integration/hermetic/visible and turnedmainred. The five split two ways, and this PR handles both — one gate reports all five, so splitting the fix buys nothing.Re-rooted: the two stdlib patches
asyncio.to_threadandos.getcwdare effectful stdlib, which the rule permits. They only tripped it because the base was reached throughdirsql._async:Both modules were already imported in their test files, and
async_mod.asynciois theasynciomodule object —patch.objectmutates that same object either way, so the patch is byte-for-byte the same target with the same scope. Nothing is broadened and the tests assert exactly what they asserted before. The neighbouringpatch.object(os.path, "isfile", ...)already used this form and already passed.Waived: the three
_RustDirSQLpatchesPatching
dirsql._async._RustDirSQLis not a shortcut around the integration tier — it is the tier's definition. AGENTS.md states it under "Test Locations":tests/integration/hermetic/mocks SQLite and the filesystem by patching the_RustDirSQLcore boundary, which is precisely why that subdir needs no native build.The rule's remedy is to redirect a first-party patch at a third-party target. There is none here.
_RustDirSQLisfrom dirsql._dirsql import DirSQL— first-party as well — and rebinding it after_asynchas already bound the name would not take effect anyway. So the rule does not describe this seam, and the two suites take a waiver rather than a refactor.The upstream schema's narrowest supported scoping for a lint rule is per-file (
linesis required forcoverage/mutationand rejected for every other rule — verified in the config reference at the sha@v0resolves to,78ab345). So: two entries, each naming one file and only the one rule, inpackages/python/testing-conventions.toml— the config this lane actually reads. No blanket file waiver, no rule-wide waiver.pathresolves against the package root (packages/python) forintegration lint's suite subjects, not against the scannedsource.Docs
AGENTS.md said "The exemption count is zero and stays there" and
agents/reference/testing-gates.mdsaid "The exemption count is again zero". Both are now false, so both are updated to name the one entry and why the rule does not apply to it, keeping the surrounding intent: an exemption is still never the way past a real gate failure — the two stdlib patches reported alongside these were re-rooted, not waived.The root
testing-conventions.tomlwas deliberately left alone: it carries the same "zero exemptions below" note, which is still true of that file, and editing it triggers the whole Rust CI lane for a comment.Verification
The gate, run at
testing-conventions0.0.104 (the localuvxcache pins 0.0.91, which predates thepatch.objectchange and reports nothing — it has to be run at a current version to reproduce):mainas-iserror: 5 lint violation(s)error: 3 lint violation(s)— exactly the three_RustDirSQLsitesThe middle row is the point: the exemption covers three sites and no more.
unit colocated-testandunit lintalso pass at 0.0.104.Suites, from
packages/python:uv run python -m pytest dirsql/ -q— 108 passeduv run python -m pytest tests/integration/ -q— 131 passedruff check ./ruff format --check .— cleanStacked on #1023
This PR is stacked on #1023 and its base is
claude/1017-ofpf-threshold, notmain. #1023 must merge first.The two gates that testing-conventions 0.0.111 added both fail on
main, and each PR fixes exactly one of them, so neither could go green alone: #1023 fixedone-function-per-filebut still failedLint integration tests; this branch fixedno-first-party-patchbut still failedCheck one function per file. Stacking is the documented way out (AGENTS.md, "PR Sizing and Issues"), and the diff-scoped gates measure this layer against the stack parent, not againstmain. GitHub retargets this PR tomainautomatically once #1023 merges.claude/1017-ofpf-thresholdis merged into this branch as an ordinary merge commit. Both branches editpackages/python/testing-conventions.toml; git merged them without conflict because the edits land in different places, and the result carries both intents:Order matters for TOML validity here (
[python]before[python.coverage]before[[python.exempt]]); the merged file parses and both settings are present.E2E Verification
N/A - test/config only
Changelog / Migrations
<root>/<pkg>/changelog.d/for each changed package (or:skip-changelogtrailer on a commit with reason)<root>/<pkg>/migrations.d/(or: not required -- additive/bugfix only)No fragment is required and none was added: the changelog gate exempts
packages/<pkg>/testing-conventions.tomland everything underpackages/<pkg>/tests/by name, which is the whole diff underpackages/. Both commits carry askip-changelog:line regardless. The merged-in #1023 content is out of this layer's diff (the gate readsbase...HEADagainstclaude/1017-ofpf-threshold) and carries its own fragment story on its own PR.PARITY.mdis unchanged -- no SDK public API is touched.Local verification of the stack
just preflight --gate one-function-per-file --gate integration-linton the merged branch: 0 failing pairs across all 11 (source, gate) pairs, testing-conventions 0.0.111. That covers both previously-red gates in both directions.