requestCount is an exact count in every runner, not a lower bound (#573)

[jeremy]

Closes #573. Third of a three-PR stack. Stacked on #595 (which is stacked on #594) — merge in order; the five unit tests added here are discovered automatically because of #594. Review this PR's own commit only.

The defect

Go, Python, Ruby, TypeScript and Kotlin all evaluated the requestCount assertion as a lower bound whenever any mock response carried Link: rel="next":

if (autoPaginates) {
    if (requestCount < expected) { fail }
} else if (requestCount != expected) { fail }

That is backwards for the fixtures it covered. conformance/tests/pagination.json queues more pages than it expects requests in two cases, because stopping early is the behavior under test:

Fixture	Queued pages	Expected requests
Pagination stops at maxPages safety cap	3	2
maxItems caps results across pages	3	2
Auto-pagination follows Link headers across multiple pages	3	3

An SDK that ignored the cap and walked all three passed 3 >= 2. The third is the exposed case: its only assertions are requestCount and noError, so an over-fetch had nothing else to catch it — the other two happen to carry a responseMeta truncated assertion that fires instead, coverage by luck.

The fix

The relaxation was protecting exactly one fixture, "List operation returns first page with Link header" (tagged link-header), whose requestCount: 1 counts first-page requests only and so cannot apply to an auto-paginating SDK. With that one fixture's count taken out of scope, nothing reaching the evaluator needs a lower bound, and >= becomes != everywhere. Swift took this in #558 and its shape is what the five now match.

The MockEngine / httptest / MSW / respx / WebMock auto-pagination tolerance stays: answering an over-walk with a terminal empty page rather than an error is what lets the tightened assertion report a clean count mismatch instead of an opaque transport error.

Each predicate moves into that language's SDK-free support module alongside delay_gaps — request_count.go, check_request_count in runner.py, RequestCount in runner.rb, request-count.ts, RequestCount.kt — with a unit test per language. Before #594, three of those five test files would have been executed by nothing.

Take the ASSERTION out of scope, not the CASE

This is the review finding that reshaped the PR. An earlier revision did it the blunt way: skip the whole link-header case in Go, Python, Ruby and TypeScript, copying what Kotlin and Swift already did. That is wrong, and wrong in this stack's own signature shape.

The fixture carries three assertions, not one:

Assertion	Applicable to an auto-paginating SDK?
requestCount: 1	No — the SDK follows the Link header
statusCode: 200	Yes
noError	Yes

Kotlin and Swift had always skipped the case, so the moment the other four joined them the fixture was skipped by all six. Its statusCode and noError assertions had been running in four runners; they then ran in zero. And nothing reports that: the fixture still sits in conformance/tests/pagination.json, still passes conformance-fixtures-check and check-fixture-coverage, so the build stays green over a fixture no runner executes. That is precisely #572's defect — present in the tree, run by nothing — one layer down, committed inside the stack that exists to close it.

So the exclusion is now one assertion wide. requestCountApplies(tags) (and its per-language spellings) returns false for link-header, the evaluator skips that one assertion, and the case runs.

Verbatim before/after from the TypeScript runner, the arm where a skip is directly observable. Pre-narrowing files taken from this stack's earlier pushed head a378d8987, everything else identical:

$ npx vitest run --reporter=verbose -t "List operation returns first page with Link header"
↓ runner.test.ts > conformance/pagination.json > List operation returns first page with Link header (TS SDK auto-paginates; follows Link headers by design)
[...]
      Tests  182 skipped (182)
REAL_EXIT=0

$ npx vitest run --reporter=verbose -t "List operation returns first page with Link header"
✓ runner.test.ts > conformance/pagination.json > List operation returns first page with Link header 19ms
[...]
      Tests  1 passed | 184 skipped (185)
REAL_EXIT=0

[...] elides vitest's per-file listing of the other 180-odd cases the -t filter skipped, plus its Test Files / Duration lines. The two quoted lines and the Tests summaries are verbatim. Zero passed becomes one passed: the case was executed by nothing and now runs.

Kotlin and Swift keep the whole-case skip, deliberately

They are not narrowed, and this is a stated exception rather than an oversight. Both derive the response status from the last mock response the SDK consumed, and an auto-paginating SDK walks past the end of a one-response queue, so statusCode has nothing to report. Narrowing them was tried; both fail:

$ make conformance-kotlin
  FAIL: List operation returns first page with Link header
        Expected status code 200, but got no response
[...]
Passed: 142, Failed: 1, Skipped: 0, Total: 143
REAL_EXIT=2

$ make conformance-swift
  FAIL: List operation returns first page with Link header
        Expected status code 200, but got no response
[...]
Passed: 142, Failed: 1, Skipped: 0, Total: 143
REAL_EXIT=2

[...] elides the other 142 result lines of each run. Note the exit code: through make a recipe failure is 2, not the runner binary's own 1.

Widening those two runners' status model is separate work, not a skip to delete here. Both call sites now carry that reasoning in a comment, so the asymmetry does not read as drift to be "aligned" away.

Net effect on coverage: the fixture goes from run by four runners (before this stack) to run by four runners (after), instead of by zero. This PR changes no skip count in any runner — see the two verification blocks below.

The general gap remains: nothing in the build detects a fixture that every runner skips. Filed as #602 and referenced from SPEC §19. This PR fixes the instance, not the class.

Red proof 1 — the runners, on the real fixture

Mutating "Auto-pagination follows Link headers across multiple pages" to expect 2 while the SDK makes 3 is exactly the over-fetch the lower bound waves through. Base is 07482b9ba, this stack's #553 tip, byte-identical to origin/main for every runner file involved.

$ make conformance-<lang>          # base 07482b9ba (lower bound)
  PASS: Auto-pagination follows Link headers across multiple pages
REAL_EXIT[base-conformance-go]=0
REAL_EXIT[base-conformance-python]=0
REAL_EXIT[base-conformance-ruby]=0
REAL_EXIT[base-conformance-typescript]=0
REAL_EXIT[base-conformance-kotlin]=0

$ make conformance-<lang>          # this PR (exact count)
  FAIL: Auto-pagination follows Link headers across multiple pages
        Expected 2 requests, got 3
REAL_EXIT[branch-conformance-go]=2
REAL_EXIT[branch-conformance-python]=2
REAL_EXIT[branch-conformance-ruby]=2
REAL_EXIT[branch-conformance-typescript]=2
REAL_EXIT[branch-conformance-kotlin]=2

The PASS/FAIL lines are one per language and identical in each; TS prints its as FAIL runner.test.ts > conformance/pagination.json > Auto-pagination follows Link headers across multiple pages with Error: [Auto-pagination follows Link headers across multiple pages] Expected 2 requests, got 3. Every exit code is real and from make, which reports a recipe failure as 2 — an earlier revision of this PR reported these as 1, which is the bare runner binary's code, not the one the stated command produces.

The fixture edit was reverted; conformance/tests/pagination.json is unchanged in this PR.

Red proof 2 — the new unit tests, against the old predicate

Restoring the lower bound in each support module fails the same three cases in every language. These are the bare test binaries, so the exit code is 1, not make's 2:

Go       --- FAIL: TestRequestCountRejectsAnOverFetch
             request_count_test.go:33: 3 requests where 2 were expected should fail;
                                       a lower bound would accept it
         --- FAIL: TestRequestCountMessageNamesBothCounts
         --- FAIL: TestRequestCountZeroExpectedRequiresZeroActual
         FAIL github.com/basecamp/basecamp-sdk/conformance/runner/go   REAL_EXIT=1
Python   3 failed, 6 passed in 0.10s                                   REAL_EXIT=1
Ruby     9 runs, 13 assertions, 3 failures, 0 errors, 0 skips          REAL_EXIT=1
TS       Tests  3 failed | 6 passed (9)                                REAL_EXIT=1
Kotlin   23 tests completed, 3 failed                                  REAL_EXIT=1
         RequestCountTest: 6 tests, 3 failures — "an over-fetch fails",
         "the failure message names both counts",
         "zero expected requires zero actual"

The three new requestCountApplies tests per language are not part of that red: they pin the SCOPE of the exclusion, asserting that a link-header fixture keeps its statusCode and noError assertions live. If the suppression ever widens back to the whole case, those fail.

Verification, exact-count code (real exit codes, measured on this commit)

make conformance-go         Passed: 141, Failed: 0, Skipped: 2   REAL_EXIT=0
make conformance-python     143 passed, 0 failed, 0 skipped      REAL_EXIT=0
make conformance-ruby       132 passed, 0 failed, 11 skipped     REAL_EXIT=0
make conformance-typescript 183 passed | 2 skipped (185)         REAL_EXIT=0
make conformance-kotlin     Passed: 142, Failed: 0, Skipped: 1   REAL_EXIT=0
make conformance-swift      Passed: 142, Failed: 0, Skipped: 1   REAL_EXIT=0
make conformance-runner-tests-go      ok (cached)                REAL_EXIT=0
make conformance-runner-tests-python  29 passed, 31 subtests     REAL_EXIT=0
make conformance-runner-tests-ruby    11 + 6 + 9 runs, 0 fail    REAL_EXIT=0
make conformance-runner-tests-kotlin  (--quiet, no output)       REAL_EXIT=0
make conformance-runner-tests-swift   39 tests, 0 failures       REAL_EXIT=0
./scripts/check-runner-test-reachability   9 checks passed       REAL_EXIT=0
./scripts/check-runner-test-reachability --self-test  6 cases    REAL_EXIT=0
./scripts/check-replay-decoder-parity  5 checks, 31 operations   REAL_EXIT=0
cd conformance/runner/go && go build ./... && go vet ./...       REAL_EXIT=0
make lint-actions           No findings to report                REAL_EXIT=0

And the same suites at base 07482b9ba — the figures the "no skip count changes" claim rests on:

base conformance-go         Passed: 141, Failed: 0, Skipped: 2
base conformance-python     143 passed, 0 failed, 0 skipped
base conformance-ruby       132 passed, 0 failed, 11 skipped
base conformance-typescript 174 passed | 2 skipped (176)
base conformance-kotlin     Passed: 142, Failed: 0, Skipped: 1

Identical skip counts throughout. TypeScript's total rises 176 → 185 because this PR adds request-count.test.ts's 9 tests; its skipped count is 2 on both sides. (Those base figures come from the red-proof-1 base runs, which carry the mutated expectation — a changed expectation moves no skip.)

Local figures — cite the CI job's own numbers where they differ.

Also in this PR

The Go runner's hasTag helper is deleted (thread). It existed only for the whole-case branch this PR no longer has, and it had been inserted between goSDKSkips' doc comment and goSDKSkips itself, so godoc read that comment as documenting hasTag.

Docs

SPEC §19's link-header entry is rewritten. Previously it was a per-runner repeat that claimed Swift's skip was "identical to Kotlin and TypeScript" — untrue, TypeScript had no link-header handling at all. It is now one block stating what each runner excludes, why Kotlin and Swift differ, and what the all-six-skipped shape would cost.

No spec files touched (spec/basecamp.smithy, openapi.json unchanged).

[Copilot]

Pull request overview

This PR fixes a false-green in the conformance harness: five of the six runners (Go, Python, Ruby, TypeScript, Kotlin) evaluated the requestCount assertion as a lower bound whenever any mock response carried Link: rel="next". That is backwards for the pagination fixtures it covered — they queue more pages than they expect requests because stopping early (at a maxPages/maxItems cap) is the behavior under test, so an over-fetching SDK could pass 3 >= 2. The fix makes requestCount an exact-equality check everywhere, matching what Swift already did in #558. The one fixture where a first-page-only count genuinely doesn't apply ("List operation returns first page with Link header") is now excluded via a link-header tag branch in all six runners, rather than special-casing the assertion.

Changes:

• Replace lower-bound requestCount logic with exact equality, extracted into per-language SDK-free helpers (request_count.go, check_request_count, RequestCount, request-count.ts, RequestCount.kt), each with a unit test.
• Add a link-header tag-based skip to the Go, Python, Ruby, and TypeScript runners (Kotlin/Swift already had it).
• Restructure SPEC §19 so the link-header exclusion is documented once as a shared architectural skip across all six runners.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
conformance/runner/go/main.go	Adds hasTag + link-header skip loop; delegates requestCount to checkRequestCount.
conformance/runner/go/request_count.go	New exact-match checkRequestCount helper.
conformance/runner/go/request_count_test.go	Unit tests for the bounds contract.
conformance/runner/python/runner.py	Adds check_request_count, tag skip; removes lower-bound branch.
conformance/runner/python/test_request_count.py	Unit tests.
conformance/runner/ruby/runner.rb	Adds RequestCount module, tag skip; removes lower-bound branch.
conformance/runner/ruby/request_count_test.rb	Unit tests.
conformance/runner/typescript/runner.test.ts	Adds tag skip; delegates to checkRequestCount; drops hasLinkNextHeader in assertions.
conformance/runner/typescript/request-count.ts	New helper.
conformance/runner/typescript/request-count.test.ts	Unit tests.
kotlin/.../conformance/Main.kt	Delegates requestCount to checkRequestCount.
kotlin/.../conformance/RequestCount.kt	New helper.
kotlin/.../conformance/RequestCountTest.kt	Unit tests.
SPEC.md	Documents the shared link-header architectural skip across all six runners.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (1)

conformance/runner/go/main.go:210

• The new hasTag function was inserted between the // Tests where the Go SDK's behavior intentionally differs. comment and the goSDKSkips declaration it documents. As a result, that comment now reads as hasTag's doc comment (in Go, a comment immediately preceding a declaration is its godoc), while goSDKSkips is left undocumented. Moving hasTag above the comment keeps the comment attached to goSDKSkips.

// Tests where the Go SDK's behavior intentionally differs.
// hasTag reports whether the fixture carries the given tag.
func hasTag(tc TestCase, tag string) bool {

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.