Repository navigation
ci: six shards, a slow marker with a nightly run, and sharded release verification - #1006
Merged
Merged
Conversation
…run for it The slowest of CI's five shards took about 11 of its 15 minutes on main (Test steps of 311, 405, 623, 508 and 538 seconds, about 40 minutes in all), and #964, which adds roughly 45% more test time, had a shard cancelled at the cap. The partition is computed from the collection and measured seconds, so a sixth shard needs no other change to conftest.py; the matrix, the shard count and the coverage job's expected fragment count move together and a test holds them equal. Simulated from tests/shard_seconds.json, each of six shards carries 1,846 weighted seconds against 2,215 for each of five. Register a `slow` marker. Pull-request shards select `-m "not perf and not slow"`; a new Slow Tests workflow runs `-m slow` nightly and on dispatch with the suite job's checkout and locked install, treating pytest's exit status 5 (nothing collected, today's state) as success and every other non-zero status as failure. Release verification keeps the full selection. The coverage job now starts after a failed shard and fails by name. A skipped required check counts as passing, and the ruleset requires suite (1) to suite (3) only, so a failure in shard 4, 5 or 6 left the job skipped and the pull request mergeable. It also requires exactly one fragment per shard.
…runner timeout Both release verifications ran the whole suite on one runner inside 60 minutes, a figure derived on 2026-09-14 from a ~25 minute suite. CI's five shards now take about 40 minutes of test time on main (1.5x headroom, not 2.4x) and #964 adds roughly 45%, about 58 minutes. A timeout there fails verification after the tag exists, and a budget of 2.5x that would be one job of about two and a half hours. Run the suite the way CI does instead: a six-shard `suite` job in each of release-verify.yml and release-advisory-verify.yml, partitioned by the same conftest.py from the same tests/shard_seconds.json, selecting `-m "not perf"` so the `slow` tests CI leaves out still run here. The `tests` job stays the gate and keeps its outputs (source_sha, qualified_wheel_sha256, qualification_sha256) and every integrity property: it needs `suite`, starts after a failed shard (`!cancelled()`) so its first step can fail it by name instead of being skipped, requires exactly one evidence record per shard each naming the commit it gates, and enforces the 85% coverage floor over the combined fragments. The artifact job is unchanged and still needs `tests`. Budgets are projected from CI's timings: 35 minutes per shard (about 14 projected, 2.5x) and 10 for the gate (about 3 projected). The runbook records both, and the derivation; replace them with a finished rehearsal's figures. Tests follow the move (the suite step is found in `jobs.suite`), pin the shard count in all three matrices and both fragment counts, hold the two release files' `suite` jobs identical, and run the gate's shell against built evidence (failed, skipped and cancelled results, a missing or surplus shard, a wrong commit, and real coverage fragments that clear the floor only together).
Replace the projected shard and gate budgets with what the first rehearsals of the sharded layout measured on 2026-10-09: Advisory Release Rehearsal run 37871044408 took 290-542 s per shard job (slowest 9.0 min) and 42 s for the gate; Release Rehearsal run 37869869033 reached the qualification download in about 50 s and stopped there, because this repository sets no qualification variables. The 35- and 10-minute timeouts are unchanged; re-derive them from the first rehearsal after #964 lands.
pengfei-threemoonslab
added a commit
that referenced
this pull request
Oct 9, 2026
Bring in #1006 (six shards, the slow marker and sharded release verification). The slow marker description now covers both the framework-matrix cases this branch marks and the nightly/release runs.
pengfei-threemoonslab
added a commit
that referenced
this pull request
Oct 9, 2026
pengfei-threemoonslab
added a commit
that referenced
this pull request
Oct 9, 2026
…p checks (#964) * fix(readers): bound constructor analysis and preserve incomplete binding evidence * fix(readers): prove ownership of #985 object-bound tools in constructor checks (#964) The constructor-ownership proof predates #985 and refused every framework object a tool list can hold as an opaque use of the framework namespace, which withheld the whole agent. Each shape is now accepted by its role, resolved by import identity, and leaves an owner obligation the list reader discharges: - ADK built-ins (google_search, load_memory, built_in_code_execution) as a member of tools=, or of a list that is followed to its uses - ADK McpToolset with its connection-parameter class (data arguments only); the toolset's import path is not its identity - hosted tools, MCP servers (also as the target of an async with) and AgentTool as the documented call with data arguments; SDK function_tool(f) - an agent held by AgentTool(agent=...) or exposed by agent.as_tool(...), across modules and through a helper returning it from its parameter - eager annotations of the agent class or a tool object on an undecorated function, when no module of the census reads annotation dictionaries - an inert dataclass of environment defaults and its instance (a new instance_data owner), os.environ[...] read into a local or a tool object's configuration, and a method call whose result is dropped on an Agent instance - a name an installed package provides is not shadowed by a sibling package of the module's own (src/app/agents is app.agents, not agents) Exports of a handle are read through their importers' uses only for a plain from-import and only for agent, instance, tool-object and wrapped-function owners. Adversarial controls for each route are in test_object_tool_ownership. * fix(readers): an execution alias another module imports is not an unused import (#964) _unused_reflection_import discharged a builtins/inspect import when only the importing module's own Name loads were checked. A module that does 'from builtins import eval as execute' and is imported by a sibling that calls execute(...) was therefore read as free of executable machinery, and a returned-tool renamed through it was silent. The discharge now also requires that no other module of the read scope imports the module (from m import name, import m [as x], from m import *, or through a re-export chain), using the caller census's own retaining set; a census cut short does not discharge. Covers the __all__ + star export and three bridge-module re-export routes. * fix(diff): report a remote MCP binding change while a graph is partial, and withhold only absence (#964) A remote McpToolset is never statically enumerable, so its graph is always partial, and the binding-comparison gate switched the whole capability comparison off: an ADK endpoint moved from a read-only host to an admin one (#538) was 'broadened' on main and silent here. Two causes. - The ownership proof refused 'os.environ[...]' inside connection-parameter data. A read that is data of an exact ADK or mcp connection class is now accepted, following enclosing calls only through such classes up to an accepted toolset's connection_params, including the nested StdioConnectionParams(server_params=StdioServerParameters(env={...})) form. Every other placement of the read stays an unread handle. - The gate. A toolset whose own construction was read and whose leaves cannot be listed now carries its own issue identity (source framework_toolset_inventory); it, and the graph_status:partial it alone causes, are not binding-graph incompleteness. And when a graph really is incomplete the gate withholds only what depends on absence (removed and narrowed rows and members, the controls of an absent tool, resolved findings) and still reports added, changed, broadened and unknown-direction rows. The binding diff stays withheld, and the requested-comparison gap still stands, found through the diff notes. Consumers checked: tool_surface compute, the binding diff in sanitization, release_decision's incomplete-comparison gap, the capability_change projection and the verify capability review notes. * fix(readers): a matched ADK sub-agent behind a constructor gap is not 'unresolved' (#964) The constructor-gap pass copied every already-matched sub-agent name into unresolved_sub_agents, which made the binding graph say the scan 'could not match' that agent to a definition. The names were read; the record's 'unread' already makes their edge incomplete, and the constructor issue is its own partial. Stop the copy, and bring the three stale list-expression assertions to the contract the PR chose: readable candidate edges survive a constructor gap, incomplete, with the constructor issue named (#964) * test(readers): name the import-search refusal the sys.modules cases now stop at (#964) Two imported-tool review cases that store sys itself into the module table are still withheld as constructor candidates, but the import-search check reads the store first. Their expected reason is now that check's text; the namespace-read case keeps the reflective-access reason. * fix(readers): decide provider absence from the listing, not the host filesystem (#964) _unprovided_root checked the absence of operator / operator.py with lstat, which finds Operator.py on a case-insensitive filesystem (macOS) and not on a case-sensitive one (Linux): the same tree was a proof of absence on one and not the other, and three tests failed only on hosted Linux. Absence now reads the exact directory listing the resolver already holds: any entry that case-folds to the provider or provider.py blocks, on every platform. The same fix is applied to the other probes whose answer depended on the host: a missing import-root component beside a differently cased entry, the above-scope namespace directory, and the pyvenv.cfg marker that decides whether a directory is read at all (and its currency check). * test(readers): track the namespace-write precision gap as strict xfails (#964) 120 parametrizations across 15 test functions in test_builder_bindings.py expect a clean read of a namespace write the ownership proof does not yet attribute to an unrelated attribute, another module or a fresh/foreign dictionary. The reader refuses them, fail-closed, which is an over-refusal and never a missed change. The precision is not implemented in this PR; it is tracked in #1005, and exactly the failing parametrizations are strict xfails so closing the gap is noticed. No refusing parametrization is marked. * perf(readers): compute each import-search answer once per read (#964) Over half of reader time in a constructor-ownership test was the import-search check (_no_import_patch -> _import_search_limit -> _import_search_issue), recomputed for every construction: about 90 calls a case, mostly for the same runner list or the same module. The answer is now remembered within one resolver read, by runner list (path and parsed tree) for _import_search_limit and by (parsed tree, sys/site carrier set) for the per-module refusal. Nothing is stored once any read bound has been reached, for a refusal that is itself a bound, or when a stop was raised, so a limit is met again rather than remembered. Behavior is unchanged: a plugin that recomputed every call with empty caches and compared it with the remembered answer found no difference across 113,669 calls in the reader test files. * test(ci): run one framework's full matrix per PR and the other's covering subset (#964) The Agents SDK and Google ADK readers share one constructor-ownership engine; a test's framework argument selects the reader. Crossing it with a route matrix (patch x layout x change x factory, ...) ran the same engine over the same routes twice, in the per-PR run. Per test function in the 20 engine test files, the ADK matrix stays whole and the Agents SDK keeps a covering subset: every value of every parameter is met at least once on the SDK reader, so each route and each reader-specific branch is still exercised there. The other SDK cases (1,291 of 22,803) are marked slow at collection (tests/framework_matrix.py, applied from conftest before -m), which the per-PR run excludes and the nightly run executes with -m slow. Nothing is deleted, and the marks are added at collection so the parameter ids other tests key on do not change. Registers the slow marker in pyproject. * docs(changelog): describe the partial-graph comparison and reader fixes (#964) Amends the existing #964 entry and the two README/quickstart sentences that said an incomplete binding graph withholds the whole capability comparison. * test(ci): re-measure shard weights and fail on an unmeasured test file (#964) tests/shard_seconds.json is re-measured from one CI-mirror run (-n auto, not perf and not slow, 21,522 cases, 424 files): the PR's new files had no weight and were guessed at by item count, which put git-fixture files in a shard as if they were pure and overran the 15-minute cap. At the measured weights the suite packs into 5 equal shards of about 3,573 weighted seconds each, or 6 of about 2,978. test_shard_partition.py now fails when a file the CI suite collects has no measured weight, naming the re-measure command; the two files that run outside the sharded suite are an explicit allow-list. The unsharded collection is taken once per process instead of twice. * perf(readers): share one importer census per read (#964) The 'does another module import this one' question asked by the unused reflection-import discharge built a fresh caller census (directory inventory and source reads) for each module that asked. One census per resolver answers all of them (#964) * test(ci): re-measure shard weights on the merged tree (#964) Measured from one CI-selection run (not perf and not slow) of cbff1ef, which adds #1006's two test files the guard named as unweighted. Six simulated shards weigh 2,468-2,489 s each. * test(readers): expect a differently cased import root to refuse on every host (#964) test_operator_receiver_proof_reads_import_roots_above_the_caller_scope derived its expectation from the host filesystem, so it contradicted test_builder_calls' case providers once absence was decided from the listing with case folded: it passed on macOS and failed on Linux CI (suite (6), case_root and case_provider). Both now expect the same host-independent refusal. Checked under a simulated case-sensitive filesystem, which reproduces CI's four failures with the old expectation.
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.
Why
Measured on
main's latest CI run, Test-step seconds forsuite (1..5)were 311 / 405 / 623 / 508 / 538, which is 2,385 s (about 40 min) in all. The slowest job took about 11 of its 15 minutes (about 74%). #964 adds roughly 45% more test time, and its last run had shard 1 cancelled at the cap. That PR is trimming its own tests and re-measuringtests/shard_seconds.json, but the margin stays thin either way.release-verify.ymlandrelease-advisory-verify.ymlran the whole suite on one runner undertimeout-minutes: 60, derived on 2026-09-14 from a ~25 minute suite. That derivation is stale. Main already needs about 40 minutes there (1.5x headroom, not 2.4x), and with #964 about 58, which leaves none. A timeout in that job fails verification after the tag exists.What changed
1. Six shards in CI. The matrix,
SHIPGATE_TEST_SHARDS, and thecoveragejob's expected fragment count are the places the count lives.conftest.pylearns the total fromSHIPGATE_TEST_SHARDS, so it needed no change. The partition is computed from the collection andtests/shard_seconds.json.tests/test_shard_partition.pynow also parametrizes six shards. Stale "four/three" wording in the comments is fixed.The
coveragejob's checks are not weakened. It still fails on zero fragments and still gates--fail-under=85over the combined data. It now also requires exactly one fragment per shard (EXPECTED_FRAGMENTS), because a gate that combined five of six would report the coverage of five sixths of the suite and could still pass.2.
slowmarker + nightly.slowis registered inpyproject.toml.--strict-markersis not used, but an undeclared marker is a typo waiting to happen.-m "not perf and not slow".slow-tests.ymlrunspython -m pytest -n auto -m slowon a daily cron and onworkflow_dispatch. It uses thesuitejob's checkout (with tags), pinned action SHAs and locked install, withpermissions: contents: read.pythonfor each status.-m "not perf", so it runsslow. A test pins all three sides of that bargain.slowin this PR.3. Release verification: sharded, not a bigger timeout. I chose the sharded restructure over re-deriving the timeout.
Re-deriving was feasible. The runbook's 2.5x rule applied to about 58 minutes gives roughly 145 minutes. That would be one job whose hung-step signal is a two-and-a-half-hour wait, and it would need re-deriving again at the next growth step. Sharding is how CI already solved the same problem, and it reuses CI's proven machinery, so release verification runs the shards CI approved.
The restructure was proportionate rather than risky because the existing tests already follow CI's earlier move of its suite into a sharded job.
suitejob in both files, key for key identical (a test holds them equal). It has six shards with the same checkout (tags), install and lock check as CI. It runs-m "not perf"plus--cov, with no per-shard floor.testsis still the gate and keeps its outputs (source_sha,qualified_wheel_sha256,qualification_sha256), its policy step, and its lints, audit and static lint.artifactstillneeds: testsand is unchanged.needs: suiteplusif: ${{ !cancelled() }}, so a failed or skipped shard cannot silently skip the gate. Its first step fails the job by name unlessneeds.suite.result == 'success'.--fail-under=85over the combined fragments.suite: 35 min. The slowest of six is about 12.6 min of test with feat(readers): read returned tool factories with constructor ownership checks #964 (mean 9.6 min x 1.31, the max/mean imbalance seen on CI) plus about 1.5 min of install, so about 14. 35 is 2.5x that.testsgate: 10 min, about 3x its estimated 3.artifact: unchanged at 15.Required checks
suite (4),suite (5)andsuite (6)are not required checks today; the ruleset requires onlysuite (1..3). I did not touch the ruleset. A skipped required check counts as passing, though, andcoverageused to be skipped behind a failed shard 4, 5 or 6, so those failures left the PR mergeable. Shards 4 and 5 already had this gap. Thecoveragejob now starts after a failed shard and fails by name (if: ${{ !cancelled() }}plus a first-step result check). That covers it without a ruleset change, and it is in its own commit if you would rather drop it.Verification
ruff check .exit 0.Release, shard, workflow-metadata, packaging, docs-link and surface-parity tests pass (exit 0):
test_release_pipeline,test_release_suite_gate(new),test_slow_tests_workflow(new),test_shard_partition,test_release_advisory_pipeline,test_safety_qualification_release,test_release_engine_smoke,test_action_metadata,test_distribution_surface_parity,test_packaging,test_docs_linksand others.Real collection partitioned six ways: the union of the shards equals the whole suite for both selections (
not perf and not slowandnot perf), with no overlap and 16.8k items. A small-project test proves a file's shard does not depend on-m(so release's shard N is CI's shard N plusslow). Mutating the conftest hook to run after deselection makes it fail.The gate's shell is run in tests against built evidence: a failed, skipped or cancelled result; 5 or 7 of 6 shards; a missing fragment or record; a wrong commit; and real coverage fragments that clear 85% only when combined. The same is done for CI's
coveragejob.Simulated from
tests/shard_seconds.json(weighted seconds per shard):The partition balances the weights near-perfectly. Last CI's actual spread (311..623 s) shows those weights, measured elsewhere, do not predict wall time well. feat(readers): read returned tool factories with constructor ownership checks #964's re-measure should help more than the sixth shard alone.
This PR's own CI run is green on all required and optional checks. Test-step seconds for
suite (1..6)were 455 / 507 / 263 / 526 / 282 / 372, which is 2,405 s in all. The slowest is 526 s (8.8 min) against 623 s on five shards, and with feat(readers): read returned tool factories with constructor ownership checks #964's +45% it projects to about 12.7 min of the 15. The max/mean ratio is again about 1.31.Risk and notes
workflow_callonly). The gate's shell and the YAML structure are tested, but a Release Rehearsal and an Advisory Release Rehearsal dispatched on this branch is the real check. It is read-only and required before any tag anyway. Their timings should replace the projected budgets in the runbook. I did not dispatch them.CHANGELOG.mdis not touched. It is the GitHub Release body, and the previous CI shard changes (ci: split the suite into five shards #996, test(ci): measure the 13 unmeasured test files so shards balance #979) did not touch it.docs/distribution-surfaces.mdand its parity test are untouched.🤖 Generated with Claude Code