Skip to content

Compare what the six generators emitted, not what two of them agree on - #752

Merged
jeremy merged 4 commits into
mainfrom
gate/service-inventory-parity
Aug 17, 2026
Merged

Compare what the six generators emitted, not what two of them agree on#752
jeremy merged 4 commits into
mainfrom
gate/service-inventory-parity

Conversation

@jeremy

@jeremy jeremy commented Aug 17, 2026

Copy link
Copy Markdown
Member

Stacked on #748 (fix/python-grouped-accessors), which it is based on — they both touch SPEC Appendix F's preamble. Merge #748 first; this retargets to main after.

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 §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.

The instrument

scripts/check-service-inventory-parity reads what the generators already emitted and asserts the emissions agree. Reading output rather than reimplementing the mapping is what keeps it from being the sixth hand-copy #745 rejected. Committed generated files only — no toolchain — so it lands in the spec-gates job beside kt-check-drift.

Eight renderings, all agreeing on 53 today:

OK: cross-SDK service inventory parity — 53 services agree across 8 renderings
    (typescript, ruby, python, kotlin, swift, kotlin-accessors, swift-accessors,
     go-accessors); 3 Go carve-outs applied and still current

It also checks each accessor file against its own services directory. That is the other half: §5's roster reads accessors, so a generator that emitted a service file and no accessor would leave the roster short with every services directory agreeing the service exists.

Normalization is data

One rule per source, expressed as fields on a SOURCES entry rather than a per-SDK branch — drop-list, one suffix to strip, one identifier casing. A gate whose premise is "five hand-copies drift" cannot itself become a pile of per-SDK ifs.

Go's three divergences are data too, and are checked for staleness rather than merely applied: a fold that closes (Automation() appears) or a spelling that changes (TimesheetTimesheets) fails the gate, so a carve-out cannot quietly stop meaning anything. Same device as spec/doc-constants.json's committed marker counts.

Red proofs — the guard→case map is measured, not reasoned

The self-test builds synthetic trees by inverting the gate's own normalization, so the builder round-tripping 53 names is itself a load-bearing positive control (a builder that spelled anything differently would make every negative case fail for the wrong reason).

Each guard was neutered in a copy of the checker (mutation asserted to match exactly one site, verified on disk, tracked file never touched — git status clean throughout) and the red set recorded:

guard neutered in the copy cases that went red
the parity comparison 1, 2, 3, 9
per-source duplicate check 4
extraction floor 5
source-exists check 6
fold carve-out staleness 7
spelling carve-out staleness 8
spelling both-spellings check 10
applying the fold carve-out both positive controls
applying the spelling carve-out both positive controls

The last two rows are the measured result. Applying a carve-out is what makes Go's 51 accessors line up with the canonical 53; delete either and the real tree stops passing. A rule that exists to prevent false positives can only be pinned by a case that is supposed to pass — same shape as the explained filter in test-check-grouped-client-coverage.rb.

Case 1 is the #745 residue in its literal form: a service in the TypeScript, Ruby and Python tables and in neither Kotlin nor Swift. Case 3 is the one that anchors §5. Case 9 proves the Go carve-outs are three named divergences, not a blanket exemption.

Prose

SPEC.md's derivation rule said "five hand-maintained copies of one table with no gate comparing them". This PR falsifies that, so it says otherwise now; the <!-- @service-count --> marker beside it is untouched and markerCounts.service-count.SPEC.md still reads 7. Appendix F's preamble and sync-doc-constants.rb's note that the residue was open are updated for the same reason — a decline (or a closure) that lives only in a commit message gets re-litigated.

Verification

All under LC_ALL=C: make check-service-inventory-parity, make test-check-service-inventory-parity, make doc-constants-check, make lint-actions, and make check-gradle-serialization + its self-test (the new check-targets entries are in its walked graph). Both CI steps carry LC_ALL: C so the UTF-8-pinned reads stay proven.

Closes #747


Summary by cubic

Checks cross-SDK service inventories by comparing what the generators emitted instead of trusting agreement between Kotlin/Swift accessors alone. Previously, SPEC §5 could pass if both accessors omitted the same service; now eight renderings across six SDKs must agree, and accessor files are checked against their own service directories.

  • Adds scripts/check-service-inventory-parity and scripts/test-check-service-inventory-parity.rb, Make targets check-service-inventory-parity and test-check-service-inventory-parity, and CI steps that run both under LC_ALL=C.
  • Reads TypeScript/Ruby/Kotlin/Swift service directories, Python’s generated __init__.py barrel (not the directory), Kotlin/Swift accessors, and Go accessors; normalizes names, rejects duplicates, enforces a floor, pins reads to UTF‑8.
  • Mirrors Python’s exact webhooks_service rename instead of stripping any _service suffix to avoid false positives on legitimate names; adds a pass-shaped self-test for that case.
  • Applies Go’s three carve‑outs as data and fails when stale; also verifies each Go accessor’s name matches its returned service type.
  • Updates SPEC and scripts/sync-doc-constants.rb to reference the new gate, note Python’s barrel, and clarify that per‑SDK drift checks validate freshness per SDK while this gate enforces cross‑SDK parity.

Written for commit b43d7aa. Summary will update on new commits.

Review in cubic


Round 2 (36a6134)

Codex, stale Python modules — fixed. Python is read from its generated __init__.py barrel, not its directory. Of the five generators only Python's fails to delete outputs a mapping stopped producing (generate-services.ts:1736, generate-services.rb:344, Main.kt:65 all sweep), so a dropped Python service left its module on disk and a directory listing counted the corpse as still emitted.

It reaches CI, which I had assumed it did not and was wrong about. check-python-service-drift.sh catches it (Only in <committed>: fanfares.py, 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 already committed (verified in a throwaway repo: 0 porcelain lines, corpse still present). The generator defect and the weaker CI step are filed as #757.

Two new self-test cases, both red-proven against the pre-fix checker:

case expects pre-fix result
11. stale module on disk, absent from barrel PASS FAIL `fanfares` is emitted by python but NOT by ...
12. barrel short while directory complete FAIL passed — only the barrel reading sees it

Reverting the Python source to a directory listing turns exactly 11 and 12 red and nothing else (measured; the header's guard-to-case table is updated, including the honest note that case 11 is pass-shaped and so also goes red for any global breakage).

Copilot, false motivation prose — fixed. The claim that the per-SDK drift scripts "compare operationIds, never service names" was wrong: TypeScript, Ruby, Python and Swift regenerate-and-diff whole trees and do see service filenames; only Kotlin's fast check is operationId-only. The real gap is the axis — each validates one SDK against its own generator and can see no other SDK. Reworded in the script header, the Makefile, the workflow step, and SPEC's derivation rule (which also now names the barrel instead of claiming five directories).

Declined, with follow-ups. Codex's three client-wiring findings (:81 TypeScript client, :77 TypeScript root exports, :85 Ruby accessors) are one class — generated-but-unreachable — and a different invariant from generator-output parity. Filed as #755, which records that the exposure is two SDKs and three doors, not five: Kotlin and Swift accessors are generated, Go is already covered by this gate, Python by #748's test. Codex's :307 (per-service operation assignment) is a third invariant and a materially bigger instrument; filed separately as #756 because it shares no remedy with #755. Reasoning is written into each thread, not only here.

Copilot AI balanced review requested due to automatic review settings August 17, 2026 05:01
@github-actions github-actions Bot added the github-actions Pull requests that update GitHub Actions label Aug 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

  • .github/workflows/test.yml

Shadow mode — this check is informational only. When activated, changes to these paths will require approval from a maintainer.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a cross-SDK gate ensuring generated service inventories remain aligned across all six SDKs.

Changes:

  • Compares eight generated/service-accessor inventories with checked Go carve-outs.
  • Adds synthetic positive and negative self-tests.
  • Integrates the checks into make, CI, and documentation.

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 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
SPEC.md Documents the enforced inventory parity.
scripts/check-service-inventory-parity Implements the cross-SDK gate.
scripts/test-check-service-inventory-parity.rb Tests gate failure modes with synthetic trees.
scripts/sync-doc-constants.rb Updates derivation commentary.
Makefile Adds parity check targets.
.github/workflows/test.yml Runs both checks in CI.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/check-service-inventory-parity Outdated
Comment thread Makefile Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6db976fa45

ℹ️ 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".

Comment thread scripts/check-service-inventory-parity
Comment thread scripts/check-service-inventory-parity
Comment thread scripts/check-service-inventory-parity
@jeremy

jeremy commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6db976fa45

ℹ️ 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".

Comment thread scripts/check-service-inventory-parity
Comment thread scripts/check-service-inventory-parity
@jeremy

jeremy commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 36a61346e9

ℹ️ 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".

@jeremy

jeremy commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

Round 2 convergence state

All 7 review threads answered and resolved: 2 fixed prose (Copilot), 1 fixed with code (Codex, stale Python modules), 4 declined with reasoning and follow-up issues (#755 reachability ×3, #756 operation assignment ×1). A third issue, #757, records the root defect the stale-module thread surfaced: generate_services.py does not sweep stale outputs, and the Python CI drift step regenerates in place and so cannot see one that is already committed.

Codex: clean at matched head — Reviewed commit: 36a61346e9, which is this PR's head.

Copilot — old head, and I could not re-request it. Its review is against 6db976fa4; both of its findings are fixed in 36a61346e and resolved with replies. POST /pulls/752/requested_reviewers for copilot-pull-request-reviewer returns HTTP 422 ("Reviews may only be requested from collaborators"), so I have no way to trigger a re-review from here. Flagging rather than implying it re-reviewed: per the established policy for this repo (Copilot flaky, Codex covers), that is normally the point at which a matched-head Codex verdict plus green CI carries the merge — but the call is the merger's, not mine.

CI: rollup SUCCESS, 41 contexts, 0 failures, 0 pending. Spec Gates SUCCESS, which is where both new steps run under LC_ALL=C.

Not merging — held for review as instructed.

Base automatically changed from fix/python-grouped-accessors to main August 17, 2026 05:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (3)

scripts/check-service-inventory-parity:341

  • This diagnosis is not valid for all compared renderings. Case 3 deliberately creates a Kotlin service file/accessor mismatch while every split table still agrees, and a Go-only mismatch can be missing hand-written wiring rather than a mapping difference. Report the inventory disagreement without asserting that the split tables caused it.
  failures << "`#{name}` is emitted by #{present.join(', ')} but NOT by #{absent.join(', ')} — " \
              "the split tables disagree about whether this service exists."

scripts/check-service-inventory-parity:220

  • This footer is emitted for every failure, including a missing input, duplicate extraction, extraction-floor failure, or stale Go carve-out. In those cases it incorrectly diagnoses split-table drift and can send maintainers to the wrong place. Keep the footer neutral and let the specific failure identify whether extraction, accessor emission, carve-outs, or mappings need correction.

This issue also appears on line 340 of the same file.

  warn "  The service split is one mapping transcribed into five generator configs."
  warn "  This gate compares what they EMITTED. A name that appears in some SDKs and"
  warn "  not others means a tag mapping was updated in some tables and not the rest."

scripts/sync-doc-constants.rb:403

  • This says all six inputs are generated output, but the new checker explicitly reads Go from go/pkg/basecamp/client.go, whose service accessors are hand-written. Describe Go separately so this rationale matches the actual evidence being compared.
# scripts/check-service-inventory-parity compares the GENERATED service
# inventories of all six SDKs — which is not the sixth hand-copy rejected above,
# since it reads generator OUTPUT rather than reimplementing the mapping. It also

@jeremy
jeremy force-pushed the gate/service-inventory-parity branch from 36a6134 to e650cec Compare August 17, 2026 05:48
@jeremy

jeremy commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

Rebased onto main after #748 squash-merged (75cdc6c69), via git rebase --onto origin/main 2e7d24ddac, and the PR base is retargeted from fix/python-grouped-accessors to main. New head e650cecab; the two commits are unchanged in content, and the diff against main is the same six files — none of #748's Python wiring is duplicated in here.

Re-verified locally against post-merge main under LC_ALL=C, real exit codes read back from a log:

  • make check-service-inventory-parityREAL_EXIT=0 — 53 services agree across 8 renderings, 3 Go carve-outs applied and still current
  • ruby scripts/test-check-service-inventory-parity.rbREAL_EXIT=0, 14/14 PASS, including the two new Python-barrel cases (11: a stale module on disk is not counted as emitted; 12: a barrel omitting a service the other seven emit still fails)
  • make doc-constants-checkREAL_EXIT=0

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e650cecab0

ℹ️ 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".

Comment thread scripts/check-service-inventory-parity Outdated
Copilot AI review requested due to automatic review settings August 17, 2026 06:01
@jeremy

jeremy commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 5e3e48ab4f

ℹ️ 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".

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

scripts/sync-doc-constants.rb:402

  • This new “closed elsewhere” paragraph now contradicts line 356 in the same comment, which still says “no gate compares them to each other.” Since this PR introduces exactly that gate, update the earlier explanation too so future readers do not get opposite descriptions of the derivation's safety.
# That residue is real and is closed ELSEWHERE, not here:
# scripts/check-service-inventory-parity compares the GENERATED service
# inventories of all six SDKs — which is not the sixth hand-copy rejected above,

Comment thread scripts/check-service-inventory-parity
jeremy added 4 commits August 16, 2026 23:16
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
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
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.
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.
Copilot AI review requested due to automatic review settings August 17, 2026 06:18
@jeremy
jeremy force-pushed the gate/service-inventory-parity branch from 5e3e48a to b43d7aa Compare August 17, 2026 06:18
@jeremy

jeremy commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

Copilot's suppressed findings — all four accepted, fixed in b43d7aaed

These arrive inside the review body in a <details><summary>Suppressed comments</summary> block. They never become review threads, so they never appear in reviewThreads and never move the unresolved count — two sweeps of mine read clean straight past them. Recording the retrieval, because the obvious REST filter silently returns nothing:

gh api "repos/basecamp/basecamp-sdk/pulls/752/reviews?per_page=100" \
  --jq '.[]|select((.body//"")|test("Suppressed comments"))|.body'

The login is copilot-pull-request-reviewer[bot] in REST and copilot-pull-request-reviewer in GraphQL; filtering REST on the GraphQL spelling matches nothing, which is indistinguishable from clean. The block also regenerates per review — the fourth finding below appeared only in the 06:05 review, after the first three.

# finding disposition
1 :341 per-failure message asserts the split tables disagree fixed
2 :220 (+340) footer diagnoses mapping drift for every failure class fixed
3 sync-doc-constants.rb:403 claims all six inputs are generated output fixed
4 sync-doc-constants.rb:402 contradicts line 356 in the same comment fixed

They are one class, and it is the same class as the two per-SDK-drift threads earlier in this PR: the gate is precise about what it MEASURES and loose about what it CLAIMS. Third catch of it, which is a signal about the prose, not three separate edits.

1 and 2 — stop naming a cause. The message said "the split tables disagree about whether this service exists". My own case 3 is a standing counterexample: it builds 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. The footer had the same defect at summary level — it printed for missing inputs, duplicate extraction, extraction-floor collapse and stale carve-outs, 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. Both now report which renderings disagree and where to look, and assert nothing about why.

3 and 4 — Go is not generated output. The rationale's whole weight is "this is not a sixth hand-copy, because it reads generator OUTPUT", and that is materially weaker if one rendering is hand-written. Seven of the eight are generated; Go is the exception, having no generated per-service files. That is now stated wherever the argument is made (the script header, sync-doc-constants.rb, and SPEC), together with the reason it is acceptable — Go's accessors are still an independent statement of which services exist — and the consequence: Go alone needs three recorded carve-outs, and now the type check below. The earlier paragraph's "no gate compares them to each other" is corrected too, without weakening the case against writing a sixth copy.

Open thread :191 — Go accessor return type. Correct, fixed, resolved.

Gauges() *ReportsService compiles (both types exist), and reading only the accessor name recorded gauges, so all eight inventories agreed while Go handed callers the wrong service. Go's accessors are its inventory here, so nothing else would have caught it. Now checked as a pair; all 51 satisfy name == type stem today. I did not use a backreference in the extraction pattern: that makes a mismatch fail to extract, surfacing as "go-accessors is missing gauges" — true, and a poor description of a line spelling gauges right there.

Red proofs (pre-fix checker via SERVICE_INVENTORY_CHECKER, git show HEAD~1: into a scratch copy)

case expects pre-fix result
14. Go accessor returning another service type FAIL passedOK: ... 53 services agree across 8 renderings, the miss exactly
15. stale carve-out failure must not say "tag mapping" FAIL without that phrase "failed correctly but MISDIAGNOSED"
3. parity message must not blame the split tables FAIL without that phrase MISDIAGNOSED

15 and 3 use a new expect_fail_without helper. A gate that fails for the right reason while naming a cause it cannot observe is still a defect, and the only way to pin a diagnostic is to assert what it must not say.

Kill matrix re-measured — each new guard pins exactly one case: accessor-vs-type → 14, footer → 15, parity message → 3. (Case 15 also goes red if the carve-out staleness check is removed, since it needs that failure to exist before it can inspect how it is described. The header table says so.)

LC_ALL=C make check-service-inventory-parity test-check-service-inventory-parity doc-constants-check lint-actions → REAL_EXIT=0, 17/17 PASS, re-run after rebasing onto 1c883d8a5.

@jeremy

jeremy commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: b43d7aaed5

ℹ️ 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".

@jeremy
jeremy merged commit f878031 into main Aug 17, 2026
46 checks passed
@jeremy
jeremy deleted the gate/service-inventory-parity branch August 17, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github-actions Pull requests that update GitHub Actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nothing compares the five per-language service-split tables to each other

2 participants