Skip to content

test+ci: bulletproof the port's CI gate and add porting test-quality guardrails - #22

Merged
LukasParke merged 1 commit into
mainfrom
LukasParke/e2e-CI
Jul 30, 2026
Merged

test+ci: bulletproof the port's CI gate and add porting test-quality guardrails#22
LukasParke merged 1 commit into
mainfrom
LukasParke/e2e-CI

Conversation

@LukasParke

@LukasParke LukasParke commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

This repo is an LLM-generated port re-synced by scripts/upstream, so CI is the only mechanical thing between a generated diff and main. It was thinner than it looked, in ways that shared one failure mode: the suite could stay green while the port drifted from upstream behavior.

What I measured first

Finding Evidence
Upstream had 46 test files at the ported commit (680bceb); this port had 15 All 104 passing. Three upstream invariants had no counterpart at all.
11 of 14 test files hand-copied their own fake client (6 byte-identical) Stubs populated only id and output, omitting status, model, created_at, tool_choice — a stub was strictly more forgiving than the real API.
mypy ran on src only → 41 errors in tests Mostly unchecked Optional derefs, which is exactly where an assertion silently stops asserting.
CI tested only Python 3.11 Despite requires-python = ">=3.9.2".
verify-port was advisory on a stale premise Its comment claimed the required-API check "fails by design until the first sync lands". The verifier actually passes 31/31 symbols, 0 failures.
No concurrency, no lockfile-drift gate, no packaging check uv.lock is committed but drift was never verified.

CI changes

  • check → 3.9/3.11/3.13 matrix, fail-fast: false so a 3.9-only break can't be masked by a passing 3.13 leg.
  • New types job: mypy src tests + uv lock --check. Not matrixed — [tool.mypy] python_version pins the analysis target, so output is identical on every interpreter.
  • New build job: builds on 3.9 (oldest supported) and imports the public API from the built wheel with --isolated --no-project, proving the artifact rather than the repo.
  • verify-port is now blocking, stale comment corrected.
  • Added concurrency (PR-only cancellation — pushes to main are never cancelled) and --frozen everywhere.

Tests: 104 → 114, coverage 81% → 83.89% behind an 83% ratchet floor

New tests/_fixtures.py: make_response populates every field OpenResponsesResult requires, so a stub can no longer be more permissive than production. assert_matches_sdk_response_shape validates the builders against the generated SDK model, so a required-field change there fails loudly instead of drifting. Builders keep upstream's camelCase wire shape because that's what the port's internals consume.

Migrated 7 files off duplicated stubs (~274 net lines removed). Three new files close the HIGH-severity gaps, each porting upstream's invariant rather than its syntax:

File Invariant Why it matters
test_turn_end_race_condition.py turn.end is never silently dropped push() after complete() is a no-op; upstream's bug was completing before the pipe drained
test_tool_execution_once.py A tool runs exactly once; zero when denied Double-executing a side-effecting tool (payment, write, email) is invisible to any final-text assertion
test_mixed_manual_tool_round.py No orphaned function_call in a follow-up Providers reject it with 400 "No tool output found for function call"

Also strengthened assertions that looked like coverage and weren't: the "turn.end" in [...] membership checks became count + ordering assertions (membership passes even when turn.end fires twice or out of order), and the vacuous assert x is None if k in d else True — which is assert True on the missing branch — can now actually fail.

Porting guardrails (the durable half)

A code-only fix gets re-broken on the next sync, so the rules live in the contract:

  • .upstreamer/upstreamer.md gains a Test Parity section: 1:1 upstream test file mapping, the rejectable assertion patterns, use the shared fixtures, coverage is a ratchet, comment deliberate divergences at the assertion.
  • .upstreamer/eval.md gains a test-quality dimension plus a command to diff the two suites by file, so a gap is visible rather than inferred.
  • New .upstreamer/skills/port-test-quality/ carries the procedure, wired into the converter skill's Step 4.
  • verify.sh now reports unported upstream test files (advisory — severity is the eval's judgment), type-checks tests, and enforces the coverage floor.

Verification

All green locally, including cross-version:

Gate Result
ruff check / format --check clean
mypy src tests Success, 53 files (was 41 errors)
pytest tests/unit 114 passed
Coverage 83.89% over an 83% floor
Required public API 31/31
verify.sh PASS, 0 failures
Python 3.9 / 3.13 114 passed each
Wheel build + isolated import passes

Beyond running the suite, I mutation-tested the new assertions rather than assuming they bite: injecting a double-execution failed 3 of 4 exactly-once tests; neutering the bespoke error-injection stub failed exactly the one test guarding it. The race test is deterministic over 60 runs on 3.12 and 25 on 3.9 (event-gated, never sleep-based). I also confirmed the fuller payload changes no port behavior — partial vs full payloads produce byte-identical event sequences, so the old stubs' omissions weren't masking a bug.

Notes for review

  • The three 0%-coverage modules are kept, not deleted. All three exist upstream, and the contract mandates one Python module per upstream lib module — deleting them would be a parity regression the next sync re-creates. Documented and excluded from the floor instead.
  • One deliberate divergence: outgoing function_call_output uses snake_case call_id, not upstream's callId, because _send normalizes at the transport boundary (model_result.py:148-156). Commented at the assertion so it doesn't get "fixed" back.
  • "3.9.2" is not pinnable in Actionsactions/python-versions ships no 3.9.2 build for ubuntu-24.04, so the matrix uses "3.9" (→ 3.9.25).
  • .upstreamer/state.yaml and eval-report.md are untouched; the openrouter substrate pin is unchanged.
  • src/ changes are additive docstrings only — no port behavior touched.

Required action before this lands

Branch protection must be updated to the six required checks: check (py3.9), check (py3.11), check (py3.13), types, build, verify-port. e2e is deliberately excluded — it exits 0 without the secret, so requiring it would be a green rubber stamp on forks. Matrix job names become protection keys, so they're pinned with explicit name:.

Deliberate follow-ups, not in this PR

  • ~33 upstream test files still have no Python counterpart. verify.sh now lists them by name; the skill makes the porting loop backfill them.
  • Two e2e tests are flaky — they depend on the model volunteering a tool call without tool_choice="required" (omitted on purpose: it persists across turns and trips the 20-turn limit). Adding retries to a paid API call didn't belong here.

🤖 Generated with Claude Code


Update: PyPI release prep (second commit)

The second commit (d150dbc) makes this package publishable. Two blockers turned up, both found by checking the index rather than assuming:

1. openrouter-agent is already taken on PyPI

Owned by an unrelated third party — a Pydantic AI integration (VinnyVanGogh, v0.1.3, last released 2025-04-21). Not ours, and not safe to assume abandoned.

Distribution renamed to openrouter-agent-sdk. The import stays openrouter_agent:

pip install openrouter-agent-sdk
from openrouter_agent import call_model, tool

A PyPI name differing from the import name is normal (scikit-learn/sklearn), and keeping the import aligned with upstream avoids churning every consumer's code and every doc example. Recorded as a fixed Package Identity table in the contract so a sync doesn't "correct" it back.

2. The SDK pin was unbounded and untested at its upper range

openrouter>=0.10.2 resolved to 0.10.3 locally, but PyPI's latest is 1.1.22 — so a fresh pip install would get a major version the port had never been tested against.

Now openrouter>=1.1,<2, verified before bumping: all 114 tests and mypy pass against 1.1.22, and the shared fixtures still validate against 1.x's OpenResponsesResult (same 18 required fields).

This forces dropping Python 3.9. Every openrouter 1.x release requires >=3.10 — the SDK dropped 3.9 exactly at 1.0.0, with 0.10.8 the last 3.9-capable release:

SDK version requires_python
0.10.8 >=3.9.2
1.0.0 → 1.1.22 >=3.10

Python 3.9 reached EOL in October 2025, so requires-python = ">=3.10" and the CI matrix is now 3.10/3.11/3.13. Both new legs verified passing; 3.9 correctly refuses to resolve. The <2 bound is deliberate — an unbounded floor is how this problem happened.

The contract forbids bumping this dependency on the port's own initiative, so .upstreamer/upstreamer.md is updated in the same commit to authorize the pin and record both consequences. Otherwise the next sync reverts it or flags it as drift.

Packaging metadata

  • [project.urls] + trove classifiers — there was no repository link in the package metadata at all.
  • sdist cleanup: it was shipping the entire porting apparatus — .upstreamer/ (contract, eval prompts, skills), .github/, opencode.json, scripts/upstream. None of it helps someone building from source, and shipping the contract invites confusion about what the package is. Now just src, tests, README, PORTING, LICENSE, pyproject, changelog.
  • Confirmed py.typed actually ships in the wheel (the README claims it does).

Publish workflow

.github/workflows/publish.yaml — PyPI trusted publishing (OIDC), so no API token is stored in this repo. Manual-only, defaults to dry-run, targets testpypi or pypi.

Publishing is irreversible (a version can never be reused, even after a yank), so it re-runs verify.sh rather than trusting an earlier CI pass, checks metadata with twine check --strict, imports the built wheel in isolation, and refuses to upload a version already on the target index.

A bug I caught while testing my own guard: the version-collision check was originally a shell for loop over a command-substituted string. It failed to detect a real collision — zsh doesn't word-split unquoted variables the way bash does. It would have worked in CI (bash), but a guard that silently stops matching is worse than no guard, since it waves through exactly the re-upload it exists to prevent. Rewritten in Python and verified in all three states: collision refused, non-collision allowed, new project allowed.

Verification

verify.sh PASS (0 failures) · mypy src tests clean · 114 passed · coverage 83.89% over an 83% floor · 31/31 required symbols · twine check --strict PASSED on both artifacts · wheel imports isolated · 3.10 and 3.13 green.

Nothing is published

Two manual setup steps are required before the first release, and the workflow cannot do them for you:

  1. On PyPI — add a GitHub trusted publisher: owner OpenRouterTeam, repo python-agent, workflow publish.yaml, environment pypi. Add it as a pending publisher since the project doesn't exist yet.
  2. In repo Settings — create the pypi and testpypi environments, and set each one's deployment branch policy to main.

That branch policy is the real ref restriction. PyPI's trusted publisher pins owner/repo/workflow/environment but carries no branch claim, and workflow_dispatch runs the workflow file from whatever ref is selected — so the in-file if: guard stops accidents while the environment policy is what actually binds publishing to main.

Then: testpypi rehearsal → pypi dry-run → real publish. Documented in PORTING.md.

Branch protection note (updated)

The required checks changed with the matrix — they are now check (py3.10), check (py3.11), check (py3.13), types, build, verify-port. (check (py3.9) no longer exists.)

…guardrails

This repo is an LLM-generated port re-synced by scripts/upstream, so CI is the
only mechanical thing between a generated diff and main. It was thinner than it
looked, in ways that all shared one failure mode: the suite could stay green
while the port drifted from upstream behavior.

Measured before this change:

- Upstream had 46 test files at the ported commit (680bceb); this port had 15,
  all passing. Three upstream invariants had no counterpart at all.
- 11 of 14 test files hand-copied their own fake client (6 byte-identical).
  Those stubs populated only `id` and `output`, omitting `status`, `model`,
  `created_at`, `tool_choice` — so a stub was strictly more forgiving than the
  real API.
- `mypy` ran on `src` only: 41 errors in tests, mostly unchecked Optional
  derefs, which is exactly where an assertion silently stops asserting.
- CI tested only Python 3.11 despite requires-python = ">=3.9.2".
- `verify-port` was advisory on a stale premise. Its comment claimed the
  required-API check "fails by design until the first sync lands"; the verifier
  actually passes 31/31 symbols with 0 failures.
- No concurrency group, no lockfile-drift gate, no packaging check.

CI (.github/workflows/ci.yaml)

- `check` becomes a 3.9/3.11/3.13 matrix with fail-fast: false, so a
  3.9-specific break cannot be masked by a passing 3.13 leg. Note that "3.9.2"
  is not pinnable: actions/python-versions ships no 3.9.2 build for
  ubuntu-24.04, so the matrix uses "3.9" (resolves to 3.9.25).
- New `types` job runs `mypy src tests` plus `uv lock --check`. Not matrixed —
  [tool.mypy] python_version pins the analysis target, so output is identical
  on every interpreter.
- New `build` job builds on 3.9 and imports the public API from the built wheel
  with `--isolated --no-project`, proving the artifact rather than the repo.
- `verify-port` is now blocking, with the stale comment corrected.
- Added concurrency (PR-only cancellation; pushes to main are never cancelled),
  and `--frozen` on every sync so lockfile drift fails loudly.
- `e2e` stays non-required on purpose: it exits 0 without the secret, so
  requiring it would be a green rubber stamp on forks.

Tests

- New tests/_fixtures.py: `make_response` populates every field
  OpenResponsesResult requires, so a stub can no longer be more permissive than
  production. `assert_matches_sdk_response_shape` validates the builders against
  the generated SDK model, so a required-field change there fails loudly instead
  of drifting. Builders keep upstream's camelCase wire shape because that is
  what the port's internals consume.
- Migrated 7 files off duplicated stubs (~274 net lines removed). Bespoke stubs
  that QueuedClient genuinely cannot express (error injection, SSE sequences)
  are kept but now build payloads from the shared builders.
- Three new files close the HIGH-severity gaps, each porting upstream's
  invariant rather than its syntax:
    test_turn_end_race_condition.py  — turn.end is never silently dropped
    test_tool_execution_once.py      — a tool runs exactly once, zero when denied
    test_mixed_manual_tool_round.py  — no orphaned function_call in a follow-up
- Strengthened assertions that looked like coverage and were not: the
  `"turn.end" in [...]` membership checks became count + ordering assertions
  (membership passes even when turn.end fires twice or out of order), and the
  vacuous `assert x is None if k in d else True` — which is `assert True` on the
  missing branch — now actually can fail.
- mypy on tests: fixed the real classes (Optional derefs, lambdas returning
  None). The `tool()`-return-type friction is suppressed narrowly for tests.*
  because fixing tool.py is ported source the next sync regenerates.

104 -> 114 tests; coverage 81% -> 83.89% behind an 83% ratchet floor.

Porting guardrails (the durable half)

A code-only fix gets re-broken on the next sync, so the rules live in the
contract:

- .upstreamer/upstreamer.md gains a Test Parity section: 1:1 upstream test file
  mapping, the rejectable assertion patterns, use the shared fixtures, coverage
  is a ratchet, comment deliberate divergences at the assertion.
- .upstreamer/eval.md gains a test-quality dimension, plus a command to diff the
  two suites by file so a gap is visible rather than inferred.
- New .upstreamer/skills/port-test-quality/ carries the procedure, wired into
  the converter skill's Step 4.
- verify.sh now reports unported upstream test files (advisory — severity is the
  eval's judgment), type-checks tests, and enforces the coverage floor.

Notes

- The three 0%-coverage modules are kept, not deleted: all three exist upstream,
  and the contract mandates one Python module per upstream lib module, so
  deleting them would be a parity regression the next sync re-creates. They are
  documented and excluded from the floor instead.
- One deliberate divergence: outgoing function_call_output uses snake_case
  `call_id`, not upstream's `callId`, because _send normalizes at the transport
  boundary. Commented at the assertion so it does not get "fixed" back.
- .upstreamer/state.yaml and eval-report.md are untouched, and the `openrouter`
  substrate pin is unchanged.

Follow-ups, deliberately not in this PR: ~33 upstream test files still have no
Python counterpart (verify.sh now lists them); and two e2e tests are flaky
because they depend on the model volunteering a tool call — adding retries to a
paid API call did not belong here.

Co-Authored-By: Claude <noreply@anthropic.com>
@LukasParke
LukasParke merged commit 590c25c into main Jul 30, 2026
7 checks passed
@LukasParke
LukasParke deleted the LukasParke/e2e-CI branch July 30, 2026 23:27
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