Skip to content

Commit 945376a

Browse files
LukasParkeclaude
andcommitted
test+ci: bulletproof the port's CI gate and add porting test-quality 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>
1 parent 52b4acf commit 945376a

32 files changed

Lines changed: 1678 additions & 542 deletions

‎.github/actions/port-toolchain/action.yml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,6 @@ runs:
1414

1515
- name: Sync dependencies
1616
shell: bash
17-
run: uv sync --all-extras
17+
# --frozen: fail on uv.lock / pyproject.toml drift rather than silently
18+
# resolving something other than what was reviewed.
19+
run: uv sync --frozen --all-extras

‎.github/workflows/ci.yaml‎

Lines changed: 126 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,157 @@
11
name: CI
22

3-
# This repo had no CI. Without it the port verifier's pytest/ruff/mypy calls only
4-
# ever ran inside the sync job, so a generated PR reached review with no
5-
# independent signal. This runs the same checks on every PR and push.
3+
# This repo is an auto-generated port: `scripts/upstream` runs an LLM against
4+
# .upstreamer/upstreamer.md and opens a PR. CI is therefore the only mechanical
5+
# thing between a generated diff and main, so it gates on the things that
6+
# actually break a port — cross-version behavior, type safety in tests as well as
7+
# src, coverage that cannot silently decay, and an installable wheel.
8+
#
9+
# Required checks (branch protection):
10+
# check (py3.9) · check (py3.11) · check (py3.13) · types · build · verify-port
11+
# Deliberately NOT required: e2e — it exits 0 when the API key is absent (forks),
12+
# so requiring it would be a green rubber stamp.
613

714
on:
815
pull_request:
916
push:
1017
branches: [main]
1118
workflow_dispatch:
1219

20+
# Supersede stale runs on a PR branch. Pushes to main are never cancelled: that
21+
# would leave gaps in the main-branch signal.
22+
concurrency:
23+
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
24+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
25+
1326
jobs:
27+
# pyproject declares requires-python = ">=3.9.2", but CI used to test 3.11
28+
# only, so a 3.9- or 3.13-specific break could land unnoticed. asyncio
29+
# primitives are the real hazard here: asyncio.Condition() binds the running
30+
# loop eagerly on 3.9 and lazily on 3.13.
1431
check:
32+
name: check (py${{ matrix.python-version }})
1533
runs-on: ubuntu-latest
1634
timeout-minutes: 15
35+
strategy:
36+
# Do not let a 3.9-only failure mask a 3.13-only failure.
37+
fail-fast: false
38+
matrix:
39+
# "3.9" resolves to 3.9.25 and satisfies ">=3.9.2". The exact patch 3.9.2
40+
# is NOT pinnable: actions/python-versions ships no 3.9.2 build for
41+
# ubuntu-24.04 (16.04/18.04/20.04 only), so `python-version: "3.9.2"`
42+
# fails to install on ubuntu-latest.
43+
python-version: ["3.9", "3.11", "3.13"]
1744
steps:
1845
- uses: actions/checkout@v4
1946

2047
- uses: actions/setup-python@v5
2148
with:
22-
python-version: "3.11"
49+
python-version: ${{ matrix.python-version }}
2350

2451
- uses: astral-sh/setup-uv@v5
2552
with:
2653
enable-cache: true
2754

28-
- run: uv sync --all-extras
55+
# --frozen: fail if uv.lock is out of sync with pyproject.toml rather than
56+
# silently resolving something different from what was reviewed.
57+
- run: uv sync --frozen --all-extras
2958

3059
- name: Lint
3160
run: uv run ruff check .
3261

3362
- name: Format
3463
run: uv run ruff format --check .
3564

36-
- name: Type check
37-
run: uv run mypy src
38-
3965
# Deterministic tests only. tests/e2e needs OPENROUTER_API_KEY and skips
4066
# cleanly without it.
4167
- name: Tests
68+
if: matrix.python-version != '3.11'
4269
run: uv run pytest tests/unit -q
4370

71+
# Coverage on one leg only: three legs would triple runtime to produce the
72+
# same single number.
73+
#
74+
# Ratchet floor. Coverage may go up, never down — raise this when it rises.
75+
# Lowering it is allowed only with an explicit reason in the PR body, since
76+
# a port run that adds source without tests shows up here first.
77+
- name: Tests with coverage
78+
if: matrix.python-version == '3.11'
79+
run: >-
80+
uv run pytest tests/unit -q
81+
--cov --cov-report=term-missing --cov-report=xml
82+
--cov-fail-under=83
83+
84+
types:
85+
runs-on: ubuntu-latest
86+
timeout-minutes: 15
87+
steps:
88+
- uses: actions/checkout@v4
89+
90+
- uses: actions/setup-python@v5
91+
with:
92+
python-version: "3.11"
93+
94+
- uses: astral-sh/setup-uv@v5
95+
with:
96+
enable-cache: true
97+
98+
- run: uv sync --frozen --all-extras
99+
100+
- name: Lockfile is in sync with pyproject
101+
run: uv lock --check
102+
103+
# tests/ included on purpose: CI used to check src only, so every fake
104+
# client and payload builder in tests/ was unverified — exactly where an
105+
# Optional deref makes an assertion silently no-op. Not matrixed because
106+
# [tool.mypy] python_version = "3.9" pins the analysis target, so the
107+
# output is identical on every interpreter.
108+
- name: Type check
109+
run: uv run mypy src tests
110+
111+
# A package that imports fine from the source tree can still ship a broken
112+
# wheel (missing package data, py.typed, or a module the build excludes).
113+
build:
114+
runs-on: ubuntu-latest
115+
timeout-minutes: 15
116+
steps:
117+
- uses: actions/checkout@v4
118+
119+
# Built on the oldest supported interpreter so a wheel that only imports
120+
# on newer syntax fails here rather than for a user on 3.9.
121+
- uses: actions/setup-python@v5
122+
with:
123+
python-version: "3.9"
124+
125+
- uses: astral-sh/setup-uv@v5
126+
with:
127+
enable-cache: true
128+
129+
- name: Build wheel and sdist
130+
run: uv build --out-dir dist
131+
132+
# --isolated --no-project: install only the built wheel, with the source
133+
# tree off sys.path, so this proves the artifact rather than the repo.
134+
- name: Import the public API from the built wheel
135+
run: |
136+
set -euo pipefail
137+
wheel=$(ls dist/*.whl)
138+
uv run --isolated --no-project --with "$wheel" python -c "
139+
from openrouter_agent import call_model, OpenRouter, tool, ModelResult
140+
import importlib.metadata as md
141+
print('imported openrouter-agent', md.version('openrouter-agent'))"
142+
143+
- uses: actions/upload-artifact@v4
144+
with:
145+
name: dist
146+
path: dist/
147+
44148
# Live end-to-end tests against the real OpenRouter API: streaming, a real
45149
# tool round, approval pause/resume, lifecycle hooks, state serialization
46150
# round-trip. Costs a few cents per run (small model, short prompts).
47151
#
48152
# Warns and exits 0 when the secret is missing (e.g. PRs from forks, where
49153
# GitHub withholds secrets) instead of failing — same pattern as upstream
50-
# typescript-agent's e2e job.
154+
# typescript-agent's e2e job. That is also why it must not be a required check.
51155
e2e:
52156
runs-on: ubuntu-latest
53157
timeout-minutes: 15
@@ -62,7 +166,7 @@ jobs:
62166
with:
63167
enable-cache: true
64168

65-
- run: uv sync --all-extras
169+
- run: uv sync --frozen --all-extras
66170

67171
- name: Live e2e tests
68172
env:
@@ -74,23 +178,25 @@ jobs:
74178
fi
75179
uv run pytest tests/e2e -q
76180
77-
# Reports the port's own mechanical gate. Advisory here, BLOCKING inside the
78-
# sync job (scripts/upstream) where it gates whether state.yaml advances.
181+
# The port's own mechanical gate — the same script `scripts/upstream` runs to
182+
# decide whether .upstreamer/state.yaml may advance.
183+
#
184+
# Blocking. It was previously advisory on the premise that the required-API
185+
# check "fails by design until the first sync lands"; that is no longer true —
186+
# the verifier passes with all 31 required symbols exported and 0 failures, so
187+
# advisory would only let that regress silently.
79188
#
80-
# Advisory on purpose: the port is currently a minor version behind upstream, so
81-
# the required-API check fails by design until the first sync lands. Making that
82-
# a red required check on every unrelated PR just teaches people to ignore CI.
83-
# The signal still shows up in the job summary.
189+
# It intentionally re-runs ruff/mypy/pytest that `check` also runs: the point is
190+
# to exercise them exactly as the sync pipeline does, so CI and the port gate
191+
# cannot drift apart.
84192
verify-port:
85193
runs-on: ubuntu-latest
86194
timeout-minutes: 15
87-
if: github.event_name == 'pull_request'
88195
steps:
89196
- uses: actions/checkout@v4
90197
- uses: ./.github/actions/port-toolchain
91-
- name: Port verifier (advisory)
198+
- name: Port verifier
92199
id: verify
93-
continue-on-error: true
94200
run: |
95201
set -o pipefail
96202
./.upstreamer/scripts/verify.sh 2>&1 | tee /tmp/verify.log
@@ -104,8 +210,7 @@ jobs:
104210
if [ "${{ steps.verify.outcome }}" = "success" ]; then
105211
echo "Port is in sync with its parity floor."
106212
else
107-
echo "Parity gaps below. Expected until the port catches up to upstream —"
108-
echo "advisory here, blocking inside the sync job."
213+
echo "Parity floor broken — see the failures below."
109214
fi
110215
echo
111216
echo '```'

‎.gitignore‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ __pycache__/
44
.pytest_cache/
55
.mypy_cache/
66
.ruff_cache/
7+
# Coverage artifacts (CI writes coverage.xml; --cov writes .coverage)
8+
.coverage
9+
.coverage.*
10+
coverage.xml
11+
htmlcov/
712
dist/
813
build/
914
*.egg-info/

‎.upstreamer/eval.md‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ version behind on real behavior. That failure mode is the one to catch.
3737
port's own shape rather than upstream behavior are not parity coverage.
3838
5. Prefer reading upstream tests: they encode the behavior contract most
3939
precisely. Check the port covers the same cases.
40+
6. Diff the two test suites by file, so an unported upstream test file is visible
41+
rather than inferred:
42+
```bash
43+
ls tmp/upstreamer/upstream/packages/agent/tests/unit/*.test.ts \
44+
| sed 's|.*/||;s|\.test\.ts$||;s|-|_|g' | sort > /tmp/up.txt
45+
ls tests/unit/test_*.py | sed 's|.*/test_||;s|\.py$||' | sort > /tmp/port.txt
46+
comm -23 /tmp/up.txt /tmp/port.txt # upstream tests with no Python counterpart
47+
```
4048

4149
## Required Qualities
4250

@@ -72,6 +80,26 @@ even on no-tools stream error paths.
7280
**Compatibility helpers.** Claude/Chat conversion round-trips preserve metadata,
7381
reasoning, tool use, and unsupported content.
7482

83+
**Test parity.** Judge the tests as coverage of *upstream* behavior, not as
84+
evidence the port ran. Concretely:
85+
86+
- Enumerate upstream's test files at the target commit and check each has a Python
87+
counterpart (`foo-bar.test.ts` → `test_foo_bar.py`). List every unported file
88+
with the invariant it protects. Unported tests covering the tool loop, state,
89+
approval/HITL ordering, hooks, or streaming are **FAIL**-worthy; cosmetic or
90+
type-level ones are warnings.
91+
- Read what the new tests assert. A test that would still pass if the port
92+
diverged from upstream is not coverage. Specifically flag: membership-only
93+
assertions on event streams (no order or count), `assert x is not None` as a
94+
test's only assertion, `len(xs) > 0` where the invariant is which items,
95+
vacuous conditional asserts, and "a tool executed" where upstream asserts
96+
**exactly once**.
97+
- Flag any new hand-rolled fake client or partial response dict that bypasses
98+
`tests/_fixtures.py`. Partial stubs omit fields the real API always sends, which
99+
is how a port passes its own suite while mishandling production payloads.
100+
- Confirm the coverage floor in `.github/workflows/ci.yaml` was not lowered. A
101+
lowered floor with no stated reason is a finding.
102+
75103
**Divergences are the documented ones.** Every difference from upstream is either
76104
in the contract's Idiomatic Divergences section or recorded as a compatibility
77105
note. An undocumented divergence is a finding.

‎.upstreamer/scripts/verify.sh‎

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,23 @@ run() {
2121
echo "=== Verification: python-agent ==="
2222
echo
2323

24+
# Coverage ratchet. Must match --cov-fail-under in .github/workflows/ci.yaml.
25+
# Raise when coverage rises; never lower it to make a port run pass.
26+
COVERAGE_FLOOR=83
27+
2428
echo "-- Toolchain"
2529
if command -v uv >/dev/null 2>&1; then
26-
run "uv sync" uv sync --all-extras
30+
# --frozen: fail on uv.lock / pyproject.toml drift instead of silently
31+
# resolving something other than what was reviewed.
32+
run "uv sync" uv sync --frozen --all-extras
33+
run "lockfile in sync" uv lock --check
2734
run "ruff check" uv run ruff check .
2835
run "ruff format" uv run ruff format --check .
29-
run "mypy" uv run mypy src
30-
run "pytest" uv run pytest tests/unit -q
36+
# tests included: a fake client or payload builder with an unchecked Optional
37+
# deref is exactly how an assertion silently stops asserting.
38+
run "mypy" uv run mypy src tests
39+
run "pytest + coverage floor ($COVERAGE_FLOOR%)" \
40+
uv run pytest tests/unit -q --cov --cov-fail-under="$COVERAGE_FLOOR"
3141
else
3242
fail "uv not installed (required to build and test this package)"
3343
fi
@@ -83,6 +93,33 @@ else
8393
fi
8494
echo
8595

96+
# The suite is what makes "a version behind on real behavior" visible or
97+
# invisible, so the file-level mapping is mechanically checked. Advisory: which
98+
# gaps are acceptable is a judgment call, and .upstreamer/eval.md makes it. This
99+
# just ensures nobody has to notice the gap on their own.
100+
echo "-- Test parity with upstream (advisory)"
101+
upstream_tests="tmp/upstreamer/upstream/packages/agent/tests/unit"
102+
if [ -d "$upstream_tests" ]; then
103+
unported=""
104+
for ts in "$upstream_tests"/*.test.ts; do
105+
[ -e "$ts" ] || continue
106+
base=$(basename "$ts" .test.ts | tr '-' '_')
107+
[ -f "tests/unit/test_${base}.py" ] || unported="$unported ${base}"
108+
done
109+
if [ -z "${unported// /}" ]; then
110+
pass "every upstream tests/unit file has a Python counterpart"
111+
else
112+
count=$(printf '%s' "$unported" | wc -w | tr -d ' ')
113+
echo " NOTE: $count upstream test file(s) have no tests/unit counterpart:"
114+
for name in $unported; do echo " $name.test.ts -> tests/unit/test_$name.py"; done
115+
echo " Not a mechanical failure — see the Test Parity section of"
116+
echo " .upstreamer/upstreamer.md and let the eval judge severity."
117+
fi
118+
else
119+
echo " SKIP: no upstream checkout — test parity unchecked"
120+
fi
121+
echo
122+
86123
echo "-- No leaked TypeScript artifacts"
87124
leaked=$(find src tests -type f \( -name '*.ts' -o -name '*.js' -o -name 'package.json' \
88125
-o -name 'tsconfig*.json' -o -name 'pnpm-lock.yaml' \) 2>/dev/null)

0 commit comments

Comments
 (0)