Skip to content

ci: six shards, a slow marker with a nightly run, and sharded release verification - #1006

Merged
pengfei-threemoonslab merged 3 commits into
mainfrom
claude/ci-shard-release-budget
Oct 9, 2026
Merged

pengfei-threemoonslab merged 3 commits into
mainfrom
claude/ci-shard-release-budget

Conversation

@pengfei-threemoonslab

@pengfei-threemoonslab pengfei-threemoonslab commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Why

Measured on main's latest CI run, Test-step seconds for suite (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-measuring tests/shard_seconds.json, but the margin stays thin either way.

release-verify.yml and release-advisory-verify.yml ran the whole suite on one runner under timeout-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 the coverage job's expected fragment count are the places the count lives. conftest.py learns the total from SHIPGATE_TEST_SHARDS, so it needed no change. The partition is computed from the collection and tests/shard_seconds.json. tests/test_shard_partition.py now also parametrizes six shards. Stale "four/three" wording in the comments is fixed.

The coverage job's checks are not weakened. It still fails on zero fragments and still gates --fail-under=85 over 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. slow marker + nightly.

  • slow is registered in pyproject.toml. --strict-markers is not used, but an undeclared marker is a typo waiting to happen.
  • Pull-request shards run -m "not perf and not slow".
  • New slow-tests.yml runs python -m pytest -n auto -m slow on a daily cron and on workflow_dispatch. It uses the suite job's checkout (with tags), pinned action SHAs and locked install, with permissions: contents: read.
  • pytest exit 5 (nothing collected, which is today's state because nothing is marked yet) is success. Every other non-zero status fails. A test runs the step's script against a fake python for each status.
  • Release verification still selects -m "not perf", so it runs slow. A test pins all three sides of that bargain.
  • No test is marked slow in 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.

  • New suite job 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.
  • Each shard uploads evidence: a coverage fragment and the commit it tested.
  • tests is still the gate and keeps its outputs (source_sha, qualified_wheel_sha256, qualification_sha256), its policy step, and its lints, audit and static lint. artifact still needs: tests and is unchanged.
  • The gate is where the six shards become one claim.
    • needs: suite plus if: ${{ !cancelled() }}, so a failed or skipped shard cannot silently skip the gate. Its first step fails the job by name unless needs.suite.result == 'success'.
    • It requires exactly six evidence records, each naming the commit the gate resolved (shards check out independently six times now, so this preserves "test A and seal B" protection).
    • It enforces --fail-under=85 over the combined fragments.
  • Shard-supplied content is never echoed into the gate's log.
  • Timeouts are projected, not observed in a rehearsal.
    • 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.
    • tests gate: 10 min, about 3x its estimated 3.
    • artifact: unchanged at 15.
    • The runbook's "Re-deriving the timeout" section, the pipeline table and the pinned tests are updated, including how to add a shard in all places.

Required checks

suite (4), suite (5) and suite (6) are not required checks today; the ruleset requires only suite (1..3). I did not touch the ruleset. A skipped required check counts as passing, though, and coverage used 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. The coverage job 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_links and others.

  • Real collection partitioned six ways: the union of the shards equals the whole suite for both selections (not perf and not slow and not 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 plus slow). 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 coverage job.

  • Simulated from tests/shard_seconds.json (weighted seconds per shard):

    shards per shard vs five
    5 2,221 each
    6 1,851 each -16.7%

    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

  • The sharded release path cannot be executed by this PR's CI (workflow_call only). 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.md is 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.
  • No registered distribution surface changes, so docs/distribution-surfaces.md and its parity test are untouched.

🤖 Generated with Claude Code

…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
pengfei-threemoonslab merged commit c1c0691 into main Oct 9, 2026
15 checks passed
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
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.
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.
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.

1 participant