Derive SPEC §5's service roster from the generated accessors (#600) - #745
Conversation
github-actions
Bot
added
github-actions
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
There was a problem hiding this comment.
Pull request overview
Derives documented service counts and rosters from generated Kotlin/Swift accessors while adding drift checks for service and conformance skip documentation.
Changes:
- Updates the canonical account-scoped service roster from 46 to 53.
- Adds service-count and roster validation with negative-case tests.
- Checks SPEC §19’s skip roster against execution manifests.
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.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/test.yml |
Updates gate documentation. |
SPEC.md |
Corrects service rosters/counts and skip documentation. |
kotlin/README.md |
Updates service counts. |
python/README.md |
Removes stale counts. |
ruby/README.md |
References the canonical service set. |
scripts/check-fixture-execution.rb |
Validates the documented skip roster. |
scripts/sync-doc-constants.rb |
Derives and checks service documentation. |
scripts/test-check-fixture-execution.rb |
Tests skip-roster failures. |
scripts/test-doc-constants.rb |
Tests service-roster/count failures. |
spec/api-gaps/event-feed.md |
Removes a stale numeric delta. |
spec/doc-constants.json |
Registers new marker kinds and counts. |
swift/README.md |
Updates service counts. |
Suppressed comments (1)
python/README.md:458
- Not all generated services are currently accessible through
AccountClient: Appendix F now saysgaugesandmyNotificationsexist but neither client references them. This wording and the “full set” link obscure that user-visible gap; distinguish the wired client surface from the generated modules.
All services are accessed through an `AccountClient`, obtained via `client.for_account(account_id)`. The table below covers the common ones; see `basecamp/generated/services/` for the full set.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Round one at The gate defect. The roster reader's fail-closed rule keyed on the list marker, and a stale entry does not need a marker to read as one. The fix is not a wider marker regex. Teaching it about The boundary is now stated rather than overclaimed. Quote-free, marker-free prose still passes, and the comment says so: an entry too vague to parse is too vague to be a roster line, the failure it could hide (under-listing) is caught from the other side by a runner exclusion the roster does not list, and only a stale entry can hide in a skipped line — which has to name its case to be one. Six negative cases added, plus a positive one pinning the quote-free continuation prose the live roster relies on, since the payload rule can only be this strict because no heading or continuation line in the real block quotes a case name. The documentation claim. Wiring those two accessors is the real fix and belongs in its own change, with its own sync/async tests, moving the Appendix F row this PR just established. Declined: projecting |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
SPEC.md:310
- The
Filessplit example remains incomplete: all generator mappings also split this tag intoCloudFilesandGoogleDocuments(for example,typescript/scripts/generate-services.ts:214-215). As written, this sentence repeats the exact omission the PR is correcting and still documents only four of the six file services.
The OpenAPI spec groups operations under coarse tags (e.g., `Automation`, `Todos`, `Files`). The service generators split those tags into the `53` fine-grained services above <!-- @service-count --> using a two-table mapping: `TAG_TO_SERVICE` (tag → default service name) and `SERVICE_SPLITS` (tag → {service → [operationIds]}). For example, the `Todos` tag splits into `Todos`, `Todolists`, `Todosets`, `TodolistGroups`; the `Files` tag splits into `Attachments`, `Uploads`, `Vaults`, `Documents`. These mappings are defined in each language's generator script. They are five hand-maintained copies of one table with no gate comparing them, so they are expected to produce identical service sets and are not guaranteed to: Appendix F records where they currently do not.
|
Follow-up to the parser fix, at The payload rule closed every line the reader falls through to. A runner heading is the one line it does not: The rule now runs ahead of every other branch, which is what makes "any line carrying a quoted case name" mean any line. It can be that strict because no heading or continuation line in the live roster quotes a case name — checked against SPEC.md, not assumed. That reordering then un-pinned the marker rule, which is the more interesting part. All four of its cases carried quoted names, so the payload rule refuses them first and they would have stayed green with the marker rule deleted outright — a suite that looks thorough while testing one guard twice. Each marker now runs twice and the quote-free half is reachable by nothing else. Kill matrix, restored by
|
|
@codex review |
Stopping the patch loop on the roster reader — this needs a call, not a fifth round
I probed the reader I had just hardened, and the payload rule has a character-class tail:
That is round four on one predicate: marker widening ( The terminating instrument, and it already exists in this PR
The same move here: every non-blank line inside the roster markers must be a runner heading or a canonical The cost is documentation formatting, which is why I am not just doing it. Five of the six sections currently rely on a fallthrough for prose:
Under the whitelist each of those has to be reflowed onto its heading line, or moved outside the markers. SPEC.md already carries much longer lines than that would produce, so it is feasible — but it is a readability call on a section people actually read, and it is yours. The three options as I see them
I have taken option 3's honesty half regardless, since it is free and its absence is the specific failure this section's lineage repeated three times: the comment now states both residues explicitly instead of implying the class is shut. If you want 1 or 2, say which and I will do it. Everything else on the PR is addressed — the two Copilot findings I fixed, the one I declined with reasoning in its thread, plus the heading hole I found by attacking my own fix. Both gates are wired into CI ( |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
SPEC.md:310
- The
Filessplit is still documented as only four services, but every generator mapping includesCloudFilesandGoogleDocumentstoo (for example,ruby/scripts/generate-services.rb:66-72). This preserves the exact omission this PR is correcting; include both services in the example.
The OpenAPI spec groups operations under coarse tags (e.g., `Automation`, `Todos`, `Files`). The service generators split those tags into the `53` fine-grained services above <!-- @service-count --> using a two-table mapping: `TAG_TO_SERVICE` (tag → default service name) and `SERVICE_SPLITS` (tag → {service → [operationIds]}). For example, the `Todos` tag splits into `Todos`, `Todolists`, `Todosets`, `TodolistGroups`; the `Files` tag splits into `Attachments`, `Uploads`, `Vaults`, `Documents`. These mappings are defined in each language's generator script. They are five hand-maintained copies of one table with no gate comparing them, so they are expected to produce identical service sets and are not guaranteed to: Appendix F records where they currently do not.
SPEC.md:3649
- The unbackticked
52is another current-value claim derived from the service roster, but@service-countvalidates and rewrites only the single backticked integer. On the next service addition the writer will update53while leaving52stale, contrary to the derived-claim rule inspec/doc-constants.json:5. Avoid restating the arithmetic here.
| Kotlin | `53` — full canonical set (`ServiceAccessors.kt`, generated; §5's other source). `todos` exposes a handwritten composite subclassing the generated `TodosService`; the other 52 are the generated classes directly <!-- @service-count --> |
6d773c3 to
5a82e2d
Compare
5a82e2d to
f5d344d
Compare
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Review status: converged on everything raised, but four commits have had no bot reviewStating this plainly rather than letting a green tick imply more than it should. Copilot is quota-limited. Its last four "reviews" — Codex has not reviewed since So the last four commits are unreviewed by either bot. Since that is exactly when a PR accumulates unexamined work, I reviewed them adversarially myself instead of waiting, and it was not idle: it found three real bugs, all in
I also probed the block side rather than assuming it was fine: #746's writable roster refuses every vacuous source in Threads: 6 of 6 answered inline, including the one decline, which is argued on layering rather than scheduling and filed as #747. I am not treating unavailable reviewers as approval. If either bot comes back with a finding on the merged commit, it gets a follow-up PR. |
Review ResultNo actionable findings on the four commits called out in the trigger. The changes consistently move validation to the shared source paths reached by both
No repository changes were necessary, so I did not create a commit or pull request. Checks
|
The row records that `gauges` and `myNotifications` are generated but unwired, which this PR discovered while measuring the per-SDK surfaces. It read as a standing fact rather than an open defect, and #732 already tracks fixing it. A documented gap nobody can navigate from is how a gap becomes permanent. Go's row deliberately gets no such reference: folding `automation` and `clientVisibility` is a design decision, not a defect awaiting a fix.
Found by an independent review of this branch, and it is the sharpest finding on the PR: the fix in a2468c0 made `make generate` sync @service-count after the accessor generators, and left the roster block out of the writer entirely. The count and the roster derive from ONE source. Leaving one writable and the other not does not preserve a choice — it splits a single claim across two mechanisms that update at different times. Reproduced: append an accessor to both generated files, run --write, and it rewrites all eleven @service-count spans to `54`, exits 0, and leaves both marked blocks still enumerating 53. A tracked spec contradicting itself, produced by a green generation run, with only a later `make check` to notice. That is the failure the `generate` target's own comment argues against, one level further down. I had excluded the roster deliberately, arguing that writing it would fix an order the checker does not assert and so convert a courtesy to readers into an enforced syntax. The argument was wrong in its weighting, and the remaining half of it costs nothing: the writer emits the accessors' order, which is the alphabetical order the roster already uses, and --check still compares SETS — a hand-reordered roster passes and is normalised on the next write, never rejected. There is a case pinning exactly that. The self-test case that asserted the old behaviour is flipped rather than supplemented: it applied drift to the roster alone and required the writer to leave it. It now drifts BOTH and requires both back, because restoring the count alone is precisely the bug. Verified against the un-fixed code by returning the roster to non-writable, which fires "expected the roster restored". The three table kinds stay unwritable for the reason they always were: each carries a column only a person can write.
Independent review, standing in for the unavailable botsWith Copilot quota-limited and Codex silent across four requests, I ran an independent adversarial review of the branch rather than let five commits ship unreviewed. It confirmed the derived facts against the actual artifacts — both accessor extractions at 53, Appendix F's six per-SDK numbers, Go's two folds and singular Accepted, and it is the sharpest finding on this PR — fixed in
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c1c8c027b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Three findings from Codex on the commit that made the roster writable. All correct. **The contract said the opposite in three places.** Making a block writable is a change to what contributors are told to do by hand, and I changed the behaviour without changing any of the documents that describe it: this script's --write header still said exactly one block kind was writable, spec/doc-constants.json still called the roster "deliberately NOT writable" and argued for it, and AGENTS.md still told contributors Zero-Skip was the sole generated block. A maintainer following those would have hand-edited a roster that `make generate` now overwrites. The doc-constants.json entry keeps the old argument and says why it was wrong, since the reasoning is the useful part. **An indented roster would have been silently unindented.** scan_file accepts a block marker up to three spaces in, and this checker strips before splitting, so a roster nested in a list passed --check — while the renderer emits at column zero, so the next `make generate` would lift it out of its list and report success. Refused rather than preserved: teaching the renderer to reproduce an indentation nothing depends on would make --write maintain a shape no longer the author's to choose. This block is generated output now, and saying so is the smaller change. **The Python workaround was wrong for async users.** The README told anyone without an accessor to construct `GaugesService(account)`. `AsyncClient.for_account` returns an `AsyncAccountClient`, and the sync class calls that client's methods without awaiting them, so the documented escape hatch hands back coroutines instead of responses for the SDK's advertised async surface. Both classes exist and are exported; the README now pairs them and says why the pairing matters. The indentation case is verified against the un-fixed gate; the other two are documentation, where the check is that they now match the code.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The service split is one mapping hand-transcribed into five generator configs, and nothing compared them. The per-SDK `check-*-service-drift` scripts compare operationIds, never service names, so a service added to three tables and forgotten in the other two produced five different SDK surfaces and a fully green `make`. #745 made SPEC section 5's roster derived, but from the generated Kotlin and Swift accessors only, because those two encode account-scoping directly. Codex named the residue: those are two independent transcriptions, not two renderings of one root, so a service omitted from BOTH leaves them agreeing and certifies the old roster and the old count. Agreement between two transcriptions is not confirmation against a root. `scripts/check-service-inventory-parity` reads what the generators ALREADY EMITTED and asserts the emissions agree — five generated service directories, the two accessor files, and Go's hand-written accessors. Reading output rather than reimplementing the mapping is what keeps it from being the sixth hand-copy #745 rejected. Committed generated files only, so it needs no toolchain and lands in the spec-gates job beside kt-check-drift. All eight agree on 53 today. It also checks each accessor file against its own services directory, which is the other half: section 5's roster reads accessors, so a generator that emitted a service file and no accessor would leave the roster short with every directory agreeing the service exists. Normalization is one rule per source, expressed as fields on a SOURCES entry rather than a per-SDK branch: files to drop, one suffix to strip, one identifier casing. Go's three divergences are data too, and are checked for STALENESS rather than merely applied — a fold that closes or a spelling that changes fails the gate instead of quietly ceasing to mean anything. The self-test drives the real checker over synthetic trees built by inverting that normalization, so the builder round-tripping 53 names is itself a positive control. Ten negative cases, and the guard-to-case mapping in its header is measured, not reasoned: each guard was neutered in a copy and the red set recorded. Neutering the parity comparison turns 1, 2, 3 and 9 red; the duplicate check 4; the floor 5; the existence check 6; the two staleness checks 7 and 8; the both-spellings check 10. Neutering either carve-out APPLICATION turns both positive controls red, which is the only way a rule that prevents false positives can be pinned. SPEC.md's derivation rule said no gate compares the tables. It does now, and the sentence says so; the `@service-count` marker beside it is untouched. Appendix F's preamble and sync-doc-constants.rb's note that the residue was open are updated for the same reason. Closes #747
The service split is one mapping hand-transcribed into five generator configs, and nothing compared them. The per-SDK `check-*-service-drift` scripts compare operationIds, never service names, so a service added to three tables and forgotten in the other two produced five different SDK surfaces and a fully green `make`. #745 made SPEC section 5's roster derived, but from the generated Kotlin and Swift accessors only, because those two encode account-scoping directly. Codex named the residue: those are two independent transcriptions, not two renderings of one root, so a service omitted from BOTH leaves them agreeing and certifies the old roster and the old count. Agreement between two transcriptions is not confirmation against a root. `scripts/check-service-inventory-parity` reads what the generators ALREADY EMITTED and asserts the emissions agree — five generated service directories, the two accessor files, and Go's hand-written accessors. Reading output rather than reimplementing the mapping is what keeps it from being the sixth hand-copy #745 rejected. Committed generated files only, so it needs no toolchain and lands in the spec-gates job beside kt-check-drift. All eight agree on 53 today. It also checks each accessor file against its own services directory, which is the other half: section 5's roster reads accessors, so a generator that emitted a service file and no accessor would leave the roster short with every directory agreeing the service exists. Normalization is one rule per source, expressed as fields on a SOURCES entry rather than a per-SDK branch: files to drop, one suffix to strip, one identifier casing. Go's three divergences are data too, and are checked for STALENESS rather than merely applied — a fold that closes or a spelling that changes fails the gate instead of quietly ceasing to mean anything. The self-test drives the real checker over synthetic trees built by inverting that normalization, so the builder round-tripping 53 names is itself a positive control. Ten negative cases, and the guard-to-case mapping in its header is measured, not reasoned: each guard was neutered in a copy and the red set recorded. Neutering the parity comparison turns 1, 2, 3 and 9 red; the duplicate check 4; the floor 5; the existence check 6; the two staleness checks 7 and 8; the both-spellings check 10. Neutering either carve-out APPLICATION turns both positive controls red, which is the only way a rule that prevents false positives can be pinned. SPEC.md's derivation rule said no gate compares the tables. It does now, and the sentence says so; the `@service-count` marker beside it is untouched. Appendix F's preamble and sync-doc-constants.rb's note that the residue was open are updated for the same reason. Closes #747
#752) * Compare what the six generators emitted, not what two of them agree on The service split is one mapping hand-transcribed into five generator configs, and nothing compared them. The per-SDK `check-*-service-drift` scripts compare operationIds, never service names, so a service added to three tables and forgotten in the other two produced five different SDK surfaces and a fully green `make`. #745 made SPEC section 5's roster derived, but from the generated Kotlin and Swift accessors only, because those two encode account-scoping directly. Codex named the residue: those are two independent transcriptions, not two renderings of one root, so a service omitted from BOTH leaves them agreeing and certifies the old roster and the old count. Agreement between two transcriptions is not confirmation against a root. `scripts/check-service-inventory-parity` reads what the generators ALREADY EMITTED and asserts the emissions agree — five generated service directories, the two accessor files, and Go's hand-written accessors. Reading output rather than reimplementing the mapping is what keeps it from being the sixth hand-copy #745 rejected. Committed generated files only, so it needs no toolchain and lands in the spec-gates job beside kt-check-drift. All eight agree on 53 today. It also checks each accessor file against its own services directory, which is the other half: section 5's roster reads accessors, so a generator that emitted a service file and no accessor would leave the roster short with every directory agreeing the service exists. Normalization is one rule per source, expressed as fields on a SOURCES entry rather than a per-SDK branch: files to drop, one suffix to strip, one identifier casing. Go's three divergences are data too, and are checked for STALENESS rather than merely applied — a fold that closes or a spelling that changes fails the gate instead of quietly ceasing to mean anything. The self-test drives the real checker over synthetic trees built by inverting that normalization, so the builder round-tripping 53 names is itself a positive control. Ten negative cases, and the guard-to-case mapping in its header is measured, not reasoned: each guard was neutered in a copy and the red set recorded. Neutering the parity comparison turns 1, 2, 3 and 9 red; the duplicate check 4; the floor 5; the existence check 6; the two staleness checks 7 and 8; the both-spellings check 10. Neutering either carve-out APPLICATION turns both positive controls red, which is the only way a rule that prevents false positives can be pinned. SPEC.md's derivation rule said no gate compares the tables. It does now, and the sentence says so; the `@service-count` marker beside it is untouched. Appendix F's preamble and sync-doc-constants.rb's note that the residue was open are updated for the same reason. Closes #747 * Read Python from its barrel, and state the motivation accurately Two review findings, both correct. CODEX: a stale Python module counted as still emitted. Of the five generators, four sweep outputs the current mapping no longer produces (generate-services.ts:1736, generate-services.rb:344, Main.kt:65, and Swift regenerates whole); `python/scripts/generate_services.py` does not. So dropping a Python service leaves its module on disk, and a directory listing counts the corpse — the exact divergence this gate exists to catch, passing it. It reaches CI, which I had assumed it did not. `check-python-service- drift.sh` regenerates to a tmpdir and `diff -rq`s, so it reports `Only in <committed>: fanfares.py` and fails — verified. But the Python CI job does not run that script. It regenerates IN PLACE and tests `git status --porcelain`, which is blind to a stale file that is already committed: nothing rewrites it, nothing deletes it, so there is no diff. Verified in a throwaway repo: 0 porcelain lines, corpse still present. Python is now read from its generated `__init__.py`, which is rewritten whole every run and so cannot name a service that is no longer emitted. It also does not import `_base.py`/`_async_base.py`, so the hand-written infrastructure drops out of the reading instead of being filtered from it. The generator defect itself is #757; this only stops one gate believing a corpse. Two cases, both red-proven against the pre-fix checker, which fails 11 with "`fanfares` is emitted by python but NOT by ..." and passes 12: 11. stale module on disk, absent from the barrel -> must PASS 12. barrel short a service the directory still has -> must FAIL Case 11 is pass-shaped on purpose: nothing else can pin a fix whose whole effect is to stop a false failure. Reverting the source to a directory listing turns 11 and 12 red and nothing else, measured. COPILOT: the motivation prose was false. It said the per-SDK drift scripts compare operationIds and never service names. TypeScript, Ruby, Python and Swift regenerate-and-diff whole trees, so they do see service filenames; only Kotlin's fast check is operationId-only. The real gap is the AXIS, not the granularity: each validates one SDK against its own generator and can see no other SDK. Reworded in the script header, the Makefile and the workflow, and SPEC's derivation rule now names the barrel rather than claiming five directories. A gate whose stated motivation is false is one nobody can size later, which is the whole reason this is worth a commit rather than a footnote. Refs #757 * Mirror Python's rename exactly, instead of stripping any _service suffix Codex, correctly. Python's entry borrowed Ruby's `strip_suffix: "_service"`, and the comment said "Ruby suffixes every file, so one rule covers both". Sharing the rule is exactly the bug: it is right for Ruby and wrong for Python. `generate_services.py`'s `service_filename` renames one module and tests for it by name: snake = to_snake_case(name) if snake == "webhooks": return "webhooks_service.py" return f"{snake}.py" Every other service keeps its snake name verbatim — including one whose canonical name legitimately ends in `_service`. A blind suffix strip took such a name apart, so Python reported a service the other seven did not have and the gate failed on a mapping where every generator agreed. That direction matters. This was a FALSE POSITIVE, not a miss: a red build blocking a correct change, and an invitation to "fix" a mapping that was never wrong. Ruby keeps `strip_suffix:` because suffixing every file genuinely is its rule; Python gets `rename:`, an exact-match map applied before any suffix logic. Two instruments, because the two generators do two different things. Case 13 adds a service whose canonical name is `notification_service` across all eight renderings and requires a PASS. The roster still holds `webhooks`, so one case pins both halves: the module that IS renamed and one that merely looks like it. Red-proven against the pre-fix checker, which fails it with both sides of the split it invents: `notification` is emitted by python but NOT by typescript, ruby, ... `notification_service` is emitted by typescript, ruby, ... but NOT by python Kill matrix re-measured. Respelling the rename as `strip_suffix` turns 13 red and nothing else, which is what makes it a pin rather than a smoke alarm. Deleting the rename outright, or never applying it, turns both positive controls plus 11 and 13 red — 11 and 13 are pass-shaped and go red on any global breakage, and the header now says so rather than implying they are specific. * Say what disagreed and where to look; stop naming a cause Four Copilot findings, all correct, and one class: the gate was precise about what it MEASURES and loose about what it CLAIMS. Three of the four arrived as SUPPRESSED comments inside the review body, which never become threads and so never move the unresolved count. Two sweeps read clean past them. The fourth appeared only in the newest review, since the block regenerates each time. The per-failure message asserted "the split tables disagree about whether this service exists". Self-test case 3 is a standing counterexample: it constructs a Kotlin service file with no accessor beside it, where every split table agrees and a generator disagrees with ITSELF. And go-accessors is hand-written, so a Go-only absence is missing wiring, not a mapping difference. It now reports which renderings lack the service and where to look, and asserts nothing about why. The failure footer had the same defect at summary level: it printed for EVERY failure class — missing input, duplicate extraction, collapsed extraction, stale carve-out — and told all of them a tag mapping had drifted. Five of this suite's own cases would have sent a maintainer to the wrong file. Now neutral. sync-doc-constants.rb said this gate compares "the GENERATED service inventories of all six SDKs". Go has no generated per-service files, so that was wrong in the one place the argument turns on it: "not a sixth hand-copy, because it reads generator OUTPUT" is weaker if a rendering is hand-written. Seven of eight are generated; Go is the exception, which is exactly why it carries three carve-outs and now the type check below. The same over-claim is corrected in the script header and SPEC. Its earlier paragraph also still said "no gate compares them to each other", which this PR falsifies; it now points at the gate without weakening the argument for not writing a sixth copy. Separately, an open thread: the Go rendering recorded only accessor NAMES, so `Gauges() *ReportsService` contributed `gauges` and the gate reported parity while Go handed callers the wrong service. Both types exist, so the compiler is silent, and Go's accessors ARE its inventory here — nothing else would catch it. Now checked as a pair. All 51 satisfy name == type stem today. A backreference in the pattern would have made a mismatch fail to EXTRACT, reported as "go-accessors is missing gauges" — true, and a bad description of a line spelling `gauges` right there. Three new cases, all red-proven against the pre-fix checker: 14. Go accessor returning another service type -> pre-fix: PASSED, "53 services agree across 8 renderings", the miss exactly 15. stale carve-out failure must not say "tag mapping" -> pre-fix: "failed correctly but MISDIAGNOSED" 3. now also asserts the parity message does NOT blame the split tables 15 and 3 use a new `expect_fail_without`: the only way to hold a diagnostic honest is to assert what it must NOT say. Kill matrix re-measured — the type check pins 14 alone, the footer pins 15 alone, the parity message pins 3 alone.
* Check that an operation lands on the same service in every SDK
`SERVICE_SPLITS` maps tag -> {service -> [operationIds]} and is transcribed by
hand into five generator configs. If ONE of them routes an operation to a
different but ALREADY-EXISTING service, nothing in `make` notices: the
service-name union is unchanged so check-service-inventory-parity sees nothing,
the global operationId set is unchanged so every per-SDK check-*-service-drift
sees nothing, and the SDK compiles with its tests exercising the method wherever
it now lives. Five SDKs expose `account.cardTables.get`, one exposes
`account.cards.get`, and both are "present".
A new sibling gate rather than a second question bolted onto
check-service-inventory-parity, whose own header forbids that shape and whose
success line states exactly one quantity. None of its eight SOURCES entries is
reusable here: different inputs (method bodies, not filenames), different key
space (250, not 53), different source count.
Keyed on `(method, path)`, not operationId, because Ruby carries `operation:` on
reads only -- 125 of 250. And on a multiset-shaped key rather than the
`{service -> path set}` #756 words it as, because paths collide verb-agnostically
inside one service (`boosts` has GET and DELETE on /boosts/{}).
Named from the CLASS declaration rather than the filename, which is what keeps
the gate free of a per-SDK naming table: all five generators name the class
`#{service}Service`, so one universal rule serves where reading filenames would
mean transcribing the sibling gate's drop/strip/rename/spelling table a second
time. Name-aware rather than a grouping comparison, because a wholesale swap of
two classes' contents leaves both the blocks and the name set unchanged.
openapi.json appears for two things only: the roster each SDK must be shown to
emit, and the operationId used to label a failure. It is never consulted for
which service an operation belongs on -- that is #756's "expensive, total"
option 2 and the sixth-hand-copy shape #745 rejected. The comparison is SDK
against SDK. Because that roster is exact, there is no MIN_ floor on the per-SDK
reads: a request helper the patterns do not recognize is reported by name rather
than as a plausible-looking count.
Go is excluded, and the reason belongs on the page or it reads as a hole.
`grep -rn "SERVICE_SPLITS\|TAG_TO_SERVICE" go/` returns nothing -- Go's service
layer is hand-authored wrappers over one flat oapi-codegen client with no
service grouping, so it has no table that could disagree. It is also assigned
differently on purpose: TrashRecording is re-exposed on ten services, and
SetClientVisibility, ListLineupMarkers and the progress reports sit elsewhere.
27 operationIds would need exempting, an order of magnitude more than
GO_CARVE_OUTS, to encode intent rather than drift.
The self-test drives synthetic five-SDK trees through the real checker. Its
roster is scraped from openapi.json and the real Swift services rather than
written down, and it keys on operationId -- which the checker never extracts
from any SDK -- so the builder and the thing it drives share no reader. The
builder rotates through every request helper the checker declares a pattern
for, so a dead rule fails the positive control.
The guard-to-case mapping in its header is measured, not reasoned about. One
mutation survives and is recorded as surviving: changing the service-suffix
strip from `sub` to `gsub` turns no case red, because all five generators spell
the class identically and the strip is cosmetic.
Its own CI step, not just `make check` membership: no CI job runs the full
`make check`.
Closes #756
* Say what the key actually is, and drop a branch that cannot be reached
Two corrections found while measuring the self-test's kill matrix.
"A multiset-shaped key" was the wrong description and would have sent the next
reader looking for counting the gate does not do. The key is `(method, path)`,
asserted unique against openapi.json, so the assignment is a plain map. What the
paragraph is actually about is why the METHOD has to be in the key: a
`{service -> path set}` merges `boosts`'s GET and DELETE on /boosts/{}, and a
reassignment of one of that pair would leave the set unchanged.
`label`'s not-found branch was unreachable — it is only ever called with a key
that came out of `expected` — so it is a `fetch` now. An unreachable fallback in
a diagnostic is worse than none: it would render a plausible-looking message for
a state that cannot arise, so nobody would learn anything from seeing it.
Closes #600.
SPEC.md§5 listed 46 account-scoped services. There are 53.#600 reported five missing. It is seven:
cloudFilesandgoogleDocumentsare also absent, and were missed by the issue itself because they arrive throughSERVICE_SPLITS["Files"]rather than a new top-level tag — no PR ever "added a service" a reader could enumerate, and SPEC has never mentioned either (zero occurrences of "cloud file" or "google doc" in the repo). I corrected the issue body rather than quietly fixing past it, since #600's own argument is that a half-fix is worse than none.Source of truth
The two generated accessor files, which must agree with each other:
kotlin/…/generated/ServiceAccessors.kt—val AccountClient.<name>swift/…/Generated/AccountClient+Services.swift—public var <name>: <X>ServiceThey exist solely to hang services off
AccountClient, so they encode account-scoping directly rather than leaving it to be inferred. A disagreement between them is reported as a generator bug, never resolved by preferring one.Rejected, with reasons worth keeping.
openapi.jsonplus each generator'sTAG_TO_SERVICE/SERVICE_SPLITSis the true root, but deriving it in Ruby means a sixth hand-copy of those tables — they already exist five times, once per language, and no gate compares them to each other (check-*-service-drift.shcompares operationIds, never service names). The derivation would become the drift surface this PR removes. Per-SDK client wiring can't answer it either: it drifts by design.The gate
Two kinds in
sync-doc-constants.rb:@account-scoped-services(block, holds the roster) and@service-count(line, sharing@operation-count's ticked-int checker).These land beside #746's
@zero-skip-roster, in the seams it opened rather than in place of them:rewrite_linetakes a hash ofkind => already-computed Integer. Generate SPEC §19's Zero-Skip roster from YAML, and delete the prose reader #746 made its arguments values rather than thunks so the write loop cannot raise after earlier files are on disk; keying that by kind applies the same rule to a set of kinds, so a third ticked-int kind is an entry at the call site and still cannot reintroduce a deferred computation.@service-countreads two generated SDK files, so it is forced only when a span actually claims it, by the same marked-only rule as the rest.@account-scoped-servicesis a block kind and stays out ofWRITABLE_BLOCK_KINDS. Since Generate SPEC §19's Zero-Skip roster from YAML, and delete the prose reader #746 made one block kind writable, that is now a choice rather than a consequence, and both places that stated the old blanket "no block kind is writable" now say which reason applies: every character of this roster is derivable, so the writer could author it — but only by fixing an order the check deliberately does not assert. The roster's claim is a set; the file is alphabetical as a courtesy to readers, not as a rule this gate holds.The roster stays a comma list.
table_rowsis table-only, so reusing it means 53 rows of exactly three cells — and the only derivable column is the name. The other two would be new hand-maintained content, i.e. new drift surface, in a change whose entire point is removing drift surface.So the parser is bespoke, and its shape is pinned tight: exactly one non-empty line, every entry
lowerCamelCase, anything else an error. #736 asserted "every misreading surfaces as a mismatch, never a silent pass" in a comment and had it breached three times. Here that property is committed as cases: bullets, a wrapped line, an emptied block, a trailing comma, a duplicate entry, a second block, both accessor-file failure modes, and an extractor that matches nothing.Every new case was shown to fail against a gate with its own guard removed — 13 mutations, each restored by
cp+diff -q. Two of them initially hit the same guard (" dupes = …"is a substring of the 4-space-indented per-file check), so the roster's own duplicate guard was untested until the anchors were fixed; both now kill at distinct sites.Every stale number, not just the roster
Marked and corrected: §1's component table, §5's heading, total-surface sentence and derivation rule, Appendix B, two Appendix F rows, and the Kotlin/Swift README claims (11
@service-countspans, all gated).Reworded to restate nothing rather than marked — each earned nothing by naming a constant: §1's topology tree
(43 more services)(already self-inconsistent: 2 shown + 43 ≠ 46), §5's= 47 servicestotal and "12 coarse tags" (openapi.jsonnow has 22), the Ruby and Python README counts, andspec/api-gaps/event-feed.md's "47 → 48".Appendix F's per-SDK table was
46across the board and is now measured:automation→Lineup andclientVisibility→Recordings, spellstimesheetssingular. Capability 53/53; surface is notgauges.pyandmy_notifications.pyexist and neither client references them§5's claim that the five generator tables "produce identical service sets across SDKs" was false; it now says what actually holds.
Verification
All under
LC_ALL=C, real exit codes:ruby scripts/sync-doc-constants.rb --checkruby scripts/test-doc-constants.rbruby scripts/test-check-fixture-execution.rbruby scripts/check-fixture-execution.rbmake doc-constants-checkmake sync-api-versionThe live gate reports 53 account-scoped services (Kotlin and Swift accessors agree) and 16 skips across 6 runners, i.e. #746's Zero-Skip roster still renders byte-identically beside the new kinds.
--writeis a no-op on the clean tree — printsalready in syncand leavesgit statusclean. This is the one that mattered for the rebuild, since #746 rewrote the writer and this PR's whole point is that the writer keeps the counts current.The reconciliation is load-bearing, not decorative. Mutating the write preflight to resolve only
operation-count(the shape #746 left behind — i.e. "forgot to extend it to the new kind") kills 18 self-test cases withKeyError: "service-count"from inside the write loop. Single matching site, verified changed on disk, restored bycp+diff -q, green again after restore.Not run: full
make check, blocked in this worktree by a local Go toolchain mismatch (go1.26.5vsgo1.26.2compiling stdlib) unrelated to this change.Follow-up worth its own issue
Those five
TAG_TO_SERVICE/SERVICE_SPLITScopies have no gate comparing them to each other. Nothing here changes that — this PR checks a downstream rendering, which is why Go's and Python's divergences show up as documentation rather than as a failure.Summary by cubic
Derives SPEC §5’s account-scoped service roster and count from the generated Kotlin/Swift
AccountClientaccessors and writes both during generation, removing drift. Old: a hand-maintained list and manual counts (46). New: roster +@service-countderive from accessors (53) and are written together, so a green generate cannot contradict itself.kotlin/.../generated/ServiceAccessors.ktandswift/.../Generated/AccountClient+Services.swiftmust agree; duplicate accessors fail; missing file or empty extraction is fatal.@service-countand@operation-countreject a zero-derived value.@api-versionand@bc3-pindates must be ISO format.@account-scoped-servicesand makes it writable; the writer emits accessor order (alphabetical today) while the check compares sets. The checker refuses malformed entries (non‑lowerCamelCase), repeated names, extra/missing services, empty or multi‑line blocks, and indented roster lines (to avoid silent reformatting).automation→Lineup andclientVisibility→Recordings; Python 51 with two unwired services, linked to Python's Client exposes 51 of 53 generated services — gauges and my_notifications are unreachable #732). Fixes Swift/Kotlin README counts and clarifies Kotlin’s handwritten composites without changing the accessor set. Updates Python README to show constructing missing-accessor services for both sync and async clients. Tweaks the event‑feed note.make generaterunssync-api-versionagain after all generators so roster and counts reflect fresh accessors; contributor docs now point tomake generate(remove stale per-target lists). CI enforces the updated doc‑constants gate and marker counts.Written for commit e27db46. Summary will update on new commits.