Skip to content

refactor(core)!: drop the deprecated ThrottleOptions.scope alias (canonical pool) - #490

Merged
rejifald merged 2 commits into
mainfrom
refactor/drop-throttle-scope-alias
Jul 24, 2026
Merged

refactor(core)!: drop the deprecated ThrottleOptions.scope alias (canonical pool)#490
rejifald merged 2 commits into
mainfrom
refactor/drop-throttle-scope-alias

Conversation

@rejifald

@rejifald rejifald commented Jul 24, 2026

Copy link
Copy Markdown
Owner

What

Extracts the throttle scope→pool slice from #470 (the contract-freeze sweep) into a standalone, independently-mergeable PR.

pool has been the canonical ThrottleOptions field since the P2 rename; scope lingered as a @deprecated back-compat alias with pool ?? scope fallback reads. This is the hard break (P19): the alias is removed.

Changes

  • Remove the @deprecated ThrottleOptions.scope field (types.ts) and trim its pool JSDoc.
  • Drop the three ?? scope fallback reads — seam.ts, resilience.ts, engine.ts now read pool only (and lose their eslint-disable no-deprecated).
  • Convert the remaining deprecated scope: 'host'|'stitch' throttle examples across tests + docs to pool. The alias-parity test in throttle-host-pooling.spec.ts collapses to the canonical spelling.
  • Regression test: a new gen-openapi case pins that the emitted throttle TODO uses pool, not scope.
  • CONTRACT.md: drop the two backlog/"migration-in-progress" entries that asserted the scope alias still exists (the P6/*Ms/rateLimit aliases they sit beside are untouched — not part of this slice).

Not in scope

Tenancy scope is a different axis and is untouched. cache / cookieSession / oauth2 scope ('principal'|'app') is the tenancy concept P2 frees the word for — this PR does not rename it to tenancy (that's a separate #470 slice). The 'host'|'stitch' value space cleanly disambiguates the two.

Verification — all gates green

check:types · check:lint · check:format (pre-commit) · check:types-d (tsd) · test (core 1213 + openapi 19 incl. the new case) · check:exports · build-docs (full twoslash) (pre-push) · check:contract.

Relation to #470 / #489

Independent off main, and disjoint from #489 (query→document). After both land, #470 rebases to drop these two slices.

🤖 Generated with Claude Code

@rejifald @claude
refactor(core)!: drop the deprecated ThrottleOptions.scope alias (can…
9c63f25 
…onical pool)

Extracts the throttle scope→pool slice from #470 (the contract-freeze sweep)
into a standalone PR so it can land on its own.

`pool` has been the canonical name since the P2 rename; `scope` lingered as a
`@deprecated` back-compat alias with `pool ?? scope` fallback reads. This is the
hard break (P19): remove the alias field, drop the three fallback reads
(seam/resilience/engine), and convert the remaining deprecated `scope: 'host'|
'stitch'` throttle examples across tests + docs to `pool`. The alias-parity
test collapses to the canonical spelling; a gen-openapi regression test pins
that the emitted TODO uses `pool`, not `scope`.

Cache/cookie/oauth tenancy `scope` (`'principal'|'app'`) is a DIFFERENT axis
and is untouched. No contract-baseline change — R4 skips `@deprecated` fields,
so there was no baselined violation to clear.

BREAKING CHANGE: `ThrottleOptions.scope` (the `@deprecated` alias of `pool`,
`'stitch'|'host'`) is removed. Use `pool`. Tenancy `scope` on cache / cookie /
oauth is unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rejifald
rejifald enabled auto-merge (squash) July 24, 2026 09:21
@rejifald
rejifald merged commit 472c4a0 into main Jul 24, 2026
12 checks passed
@rejifald
rejifald deleted the refactor/drop-throttle-scope-alias branch July 24, 2026 09:27
@rejifald rejifald mentioned this pull request Jul 24, 2026
Merged
rejifald added a commit that referenced this pull request Aug 4, 2026
@rejifald @claude
fix(download,core): adopt ADR 0022, the pre-GA name sweep, and P24/R8
f9a56b5 
`claude/download-consolidate` was cut 2026-07-06 with a 2026-07-23 merge base.
Main has since landed the pre-GA breaking sweep, so replaying the seven commits
was only half the work — the other half is what the ratchets then demanded.

Core

- `download`'s `interpret` composes `verdictOf(res, cfg)` (ADR 0022 Decision 4)
  and yields `data`, not `value`. The M1 complete-body rule sits AFTER the
  composed verdict, as the surface's own, and now reads `verdict.accept`:
  `classifyStatus` consults it only at `>= 400`, so without that read a caller
  who declared `206` NORMAL would still be rejected — the surface overriding an
  explicit declaration rather than ruling where none was made. Second test in
  download-stray-206.spec.ts pins it.
- `rebuildError`'s cause-carry lands on top of main's body/url arm instead of
  replacing it.

Test rig + specs

- `throttle-host-pooling` keeps the deterministic manual-clock rewrite (the
  wall-clock flake fix) minus its `scope`-alias half — #490 removed the alias.
  ADR 0023 Decision 1 ratified exactly the 500ms in-process spacing it asserts.
- Rig duration fields drop the `Ms` suffix (D3/P17), matching main's own
  `stream.chunkDelay`: `ttfbDelayMs` → `ttfbDelay`, `chunkDelayMs` → `chunkDelay`.
  The rig keeps main's `retryAfterSeconds` (the header's native unit) over the
  branch's `retryAfter`; four existing specs already depend on it.
- Spec config: auth moved to `stitchapi/auth` (ADR 0021) and `apiKey` takes
  `secret` (#580); `retry.baseMs`/`baseDelay` fold into `retry.backoff.base`
  (#513); `respectRetryAfter` → `retry.respect` (#608); `AdapterProgress.phase`
  → `direction`.

@stitchapi/download

- `DownloadBatch.cancel`/`cancelAll` fold into `cancel(id?)` — R8 landed after
  this branch was cut and flags the shared prefix (P24). For a verb the optional
  parameter IS the collapse; an envelope would be the same pair one level deeper.
- `hooks` is built unannotated so `onError` infers REQUIRED: the slot is
  `AtLeastOne<Hooks>` now (P20, #507), which an all-optional annotation cannot
  satisfy.
- Version + vitest to the rc.7 lockstep; lockfile regenerated from main's.
- classify.ts and the README no longer claim the engine drops the transport
  cause — this branch's own core fix carries it through. The hook seam stays:
  it is strictly wider (it also catches a thrown non-`Error`).

Verify: 40/40 workspace projects green (core 1460 passed, download 15/15);
check:lint / types / types-d / exports / exports:companions / contract /
unknown-keys / release / docs-links / format all clean; core size 23.45 gzip
(budget 23.50) and 20.87 for `import { stitch }` (20.90), @stitchapi/download
2.45/2.65 KB; `yakir check` both tiers 0 drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rejifald added a commit that referenced this pull request Aug 4, 2026
@rejifald @claude
fix(download,core): adopt ADR 0022, the pre-GA name sweep, and P24/R8
fa92962 
`claude/download-consolidate` was cut 2026-07-06 with a 2026-07-23 merge base.
Main has since landed the pre-GA breaking sweep, so replaying the seven commits
was only half the work — the other half is what the ratchets then demanded.

Core

- `download`'s `interpret` composes `verdictOf(res, cfg)` (ADR 0022 Decision 4)
  and yields `data`, not `value`. The M1 complete-body rule sits AFTER the
  composed verdict, as the surface's own, and now reads `verdict.accept`:
  `classifyStatus` consults it only at `>= 400`, so without that read a caller
  who declared `206` NORMAL would still be rejected — the surface overriding an
  explicit declaration rather than ruling where none was made. Second test in
  download-stray-206.spec.ts pins it.
- `rebuildError`'s cause-carry lands on top of main's body/url arm instead of
  replacing it.

Test rig + specs

- `throttle-host-pooling` keeps the deterministic manual-clock rewrite (the
  wall-clock flake fix) minus its `scope`-alias half — #490 removed the alias.
  ADR 0023 Decision 1 ratified exactly the 500ms in-process spacing it asserts.
- Rig duration fields drop the `Ms` suffix (D3/P17), matching main's own
  `stream.chunkDelay`: `ttfbDelayMs` → `ttfbDelay`, `chunkDelayMs` → `chunkDelay`.
  The rig keeps main's `retryAfterSeconds` (the header's native unit) over the
  branch's `retryAfter`; four existing specs already depend on it.
- Spec config: auth moved to `stitchapi/auth` (ADR 0021) and `apiKey` takes
  `secret` (#580); `retry.baseMs`/`baseDelay` fold into `retry.backoff.base`
  (#513); `respectRetryAfter` → `retry.respect` (#608); `AdapterProgress.phase`
  → `direction`.

@stitchapi/download

- `DownloadBatch.cancel`/`cancelAll` fold into `cancel(id?)` — R8 landed after
  this branch was cut and flags the shared prefix (P24). For a verb the optional
  parameter IS the collapse; an envelope would be the same pair one level deeper.
- `hooks` is built unannotated so `onError` infers REQUIRED: the slot is
  `AtLeastOne<Hooks>` now (P20, #507), which an all-optional annotation cannot
  satisfy.
- Version + vitest to the rc.7 lockstep; lockfile regenerated from main's.
- classify.ts and the README no longer claim the engine drops the transport
  cause — this branch's own core fix carries it through. The hook seam stays:
  it is strictly wider (it also catches a thrown non-`Error`).

Verify: 40/40 workspace projects green (core 1460 passed, download 15/15);
check:lint / types / types-d / exports / exports:companions / contract /
unknown-keys / release / docs-links / format all clean; core size 23.45 gzip
(budget 23.50) and 20.87 for `import { stitch }` (20.90), @stitchapi/download
2.45/2.65 KB; `yakir check` both tiers 0 drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@rejifald