Skip to content

ci: fix the no-first-party-patch failures in the Python hermetic tier - #1026

Merged
thekevinscott merged 4 commits into
claude/1017-ofpf-thresholdfrom
claude/tackle-1024-nfpp
Sep 4, 2026
Merged

thekevinscott merged 4 commits into
claude/1017-ofpf-thresholdfrom
claude/tackle-1024-nfpp

Conversation

@thekevinbot

@thekevinbot thekevinbot commented Sep 2, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1024

integration lint's no-first-party-patch rule now resolves the patch.object(...) object form, which made five previously-invisible patches in packages/python/tests/integration/hermetic/ visible and turned main red. 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_thread and os.getcwd are effectful stdlib, which the rule permits. They only tripped it because the base was reached through dirsql._async:

patch.object(async_mod.asyncio, "to_thread", spy)   ->  patch.object(asyncio, "to_thread", spy)
patch.object(async_mod.os, "getcwd", return_value="/cwd")  ->  patch.object(os, "getcwd", return_value="/cwd")

Both modules were already imported in their test files, and async_mod.asyncio is the asyncio module object — patch.object mutates 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 neighbouring patch.object(os.path, "isfile", ...) already used this form and already passed.

Waived: the three _RustDirSQL patches

Patching dirsql._async._RustDirSQL is 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 _RustDirSQL core 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. _RustDirSQL is from dirsql._dirsql import DirSQL — first-party as well — and rebinding it after _async has 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 (lines is required for coverage/mutation and rejected for every other rule — verified in the config reference at the sha @v0 resolves to, 78ab345). So: two entries, each naming one file and only the one rule, in packages/python/testing-conventions.toml — the config this lane actually reads. No blanket file waiver, no rule-wide waiver.

[[python.exempt]]
path = "tests/integration/hermetic/dirsql_test.py"
rules = ["no-first-party-patch"]
reason = "the hermetic tier IS the _RustDirSQL core-boundary patch; no third-party target exists to move it to"

[[python.exempt]]
path = "tests/integration/hermetic/extensions_test.py"
rules = ["no-first-party-patch"]
reason = "the hermetic tier IS the _RustDirSQL core-boundary patch; no third-party target exists to move it to"

path resolves against the package root (packages/python) for integration lint's suite subjects, not against the scanned source.

Docs

AGENTS.md said "The exemption count is zero and stays there" and agents/reference/testing-gates.md said "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.toml was 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-conventions 0.0.104 (the local uvx cache pins 0.0.91, which predates the patch.object change and reports nothing — it has to be run at a current version to reproduce):

Tree Result
main as-is error: 5 lint violation(s)
re-rooting only, exemptions removed error: 3 lint violation(s) — exactly the three _RustDirSQL sites
this branch exit 0

The middle row is the point: the exemption covers three sites and no more. unit colocated-test and unit lint also pass at 0.0.104.

Suites, from packages/python:

  • uv run python -m pytest dirsql/ -q — 108 passed
  • uv run python -m pytest tests/integration/ -q — 131 passed
  • ruff check . / ruff format --check . — clean

Stacked on #1023

This PR is stacked on #1023 and its base is claude/1017-ofpf-threshold, not main. #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 fixed one-function-per-file but still failed Lint integration tests; this branch fixed no-first-party-patch but still failed Check 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 against main. GitHub retargets this PR to main automatically once #1023 merges.

claude/1017-ofpf-threshold is merged into this branch as an ordinary merge commit. Both branches edit packages/python/testing-conventions.toml; git merged them without conflict because the edits land in different places, and the result carries both intents:

[python]
one_function_per_file = { max_lines = 100 }   # from #1023

[python.coverage]
fail_under = 100
branch = true

[[python.exempt]]                              # from this PR
path = "tests/integration/hermetic/dirsql_test.py"
...

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

  • Changelog fragment added under <root>/<pkg>/changelog.d/ for each changed package (or: skip-changelog trailer on a commit with reason)
  • Migration fragment added under <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.toml and everything under packages/<pkg>/tests/ by name, which is the whole diff under packages/. Both commits carry a skip-changelog: line regardless. The merged-in #1023 content is out of this layer's diff (the gate reads base...HEAD against claude/1017-ofpf-threshold) and carries its own fragment story on its own PR.

PARITY.md is unchanged -- no SDK public API is touched.

Local verification of the stack

just preflight --gate one-function-per-file --gate integration-lint on 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.

`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
@thekevinbot
thekevinbot changed the base branch from main to claude/1017-ofpf-threshold September 4, 2026 19:45
@thekevinscott
thekevinscott merged commit 5b2cde5 into claude/1017-ofpf-threshold Sep 4, 2026
95 checks passed
@thekevinscott
thekevinscott deleted the claude/tackle-1024-nfpp branch September 4, 2026 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: Python integration lint is red -- no-first-party-patch now resolves the patch.object form

2 participants