feat(typecheck): public-method fixture coverage gate#3481
Open
caio-pizzol wants to merge 3 commits into
Open
Conversation
Mechanizes the rule documented in CONTRIBUTING.md after #3480: every public SuperDoc method or getter must have at least one consumer-side fixture reference, or be explicitly allowlisted with a reason. Why this exists: the SuperDoc.js -> SuperDoc.ts migration shipped `search(text: string)`, a narrowed version of the previous `string | RegExp` contract. Every existing gate passed - `check:types`, `check:public:superdoc`, the consumer matrix, the deep-type audit, 1054 tests. The bot caught it because no consumer fixture asserted Parameters<SuperDoc['search']>. The miss was: an unguarded public method can ship with wrong-but-explicit types. This gate prevents the next instance of that class. It does not guarantee the assertion is correct (a typed but wrong assertion would still pass); it guarantees a reviewer was asked to write one. Implementation matches the JSDoc ratchet shape from PR #3474: - tests/consumer-typecheck/check-public-method-coverage.mjs - walks SuperDoc.ts with the TypeScript AST, enumerates public methods + getters (skipping private, static, @internal, # prefix, and the EventEmitter inherited surface). For each, scans every fixture under tests/consumer-typecheck/src/ for one of `Parameters<SuperDoc['name']>`, `ReturnType<SuperDoc['name']>`, or `(superdoc|sd).name(`. Fails when a NEW member is uncovered (i.e. not on the debt snapshot) or when a snapshot entry is stale (now covered). - tests/consumer-typecheck/public-method-coverage-allowlist.cjs - the escape hatch for members that are intentionally not consumer- callable (8 entries today: broadcast* lifecycle relays + the internal setActiveEditor / onContentError handlers). - tests/consumer-typecheck/public-method-coverage-debt-snapshot.json - records the 33 currently-uncovered public methods. Future PRs can drain it by adding fixtures and running `--write`. - scripts/check-public-contract.mjs - new stage 4 `public-method-coverage` between jsdoc-ratchet and build. Stage count: 9 -> 10. Verified (all simulated against the actual main HEAD): - baseline -> OK 36 public members; 33 tracked as known debt - simulated new uncovered method (drop one snapshot entry) -> FAIL with "1 NEW public member(s) without any fixture reference" - simulated stale entry (add nonexistent name to snapshot) -> FAIL with "1 stale entry/entries" + `--write` refresh hint - pnpm check:public:superdoc --skip-build -> PASS 9 ran / 1 skipped, 123.4s; new stage runs as [4/10] without disturbing the rest Followup, separate PR: runtime payload tests for SuperDocEventMap events SuperDoc emits directly (cheap events first: ready, editorCreate, editorBeforeCreate).
…llowlist
Review caught three blockers in the first version:
1. The gate counted any call site as coverage, so the search regression
would have slipped through (search-match.ts has sd.search('hello')).
2. Getters had no satisfaction pattern (Parameters/ReturnType are
method-shaped); a future getter could only pass via the snapshot.
3. The allowlist contract (non-empty reason, key must match a real
member) was unvalidated.
Rewrote the gate as obligation-based, not mention-based:
For each public member, compute REQUIRED obligations from the AST:
- method with >=1 param → requires "parameters"
- method with non-void return → requires "returns"
- getter → requires "returns"
- zero-param void method → requires "call"
(otherwise renaming would slip past silently)
Satisfaction patterns:
- parameters → Parameters<SuperDoc['name']>
- returns (method) → ReturnType<SuperDoc['name']>
- returns (getter) → SuperDoc['name'] or typeof sd.name
- call → (superdoc|sd).name(
**Call sites no longer satisfy parameters/returns** - they only
satisfy the "call" obligation. This is the central fix for the
search regression class.
The debt snapshot now tracks obligation-level entries like
"state:returns" instead of just member names, so partial coverage
is visible.
Allowlist validation: each key must match an actual public member
of SuperDoc; each value must be a non-empty string. Empty reasons,
typos, and stale keys all fail with specific messages.
Snapshot count: 33 (member-level) → 59 (obligation-level), reflecting
the actual unmet obligation surface.
Verified:
- Baseline: OK 75 obligations across 36 members, 59 tracked as debt.
- Sim search regression (remove Parameters<SuperDoc['search']>) →
FAIL with "+ search:parameters". Proves the gate now catches the
bug class it was built for.
- Sim stale snapshot entry → FAIL with --write hint.
- Sim allowlist with empty reason → FAIL with "missing or empty reason".
- Sim allowlist key not a member → FAIL with "not a public member of
SuperDoc (typo or stale entry)".
- pnpm check:public:superdoc --skip-build → PASS 9 ran / 1 skipped,
122.5s; new stage runs as [4/10].
…ew gate After adding the public-method-coverage stage: - AGENTS.md: nine → ten stages; added `public-method-coverage` to the staged list and to the umbrella description. - packages/superdoc/scripts/README.md: new row for `check-public-method-coverage.mjs` in the consumer-typecheck infrastructure table (snapshot + allowlist paths, refresh command, the call-site distinction); updated the stage-count sentence (five → six wrapper stages of check:public:superdoc after the policy gates) and added public-method-coverage to the cheap-policy-gate list. - scripts/check-public-contract.mjs: tightened both the stage 4 docstring and the stage `blurb` to use 'obligation' language consistently. The blurb explicitly says call sites do NOT satisfy parameters/returns on their own — the central fix that separated v2 from v1.
![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: 6ac67cf395
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+168
to
+169
| const files = readdirSync(FIXTURE_DIR).filter( | ||
| (f) => f.endsWith('.ts') || f.endsWith('.cts') || f.endsWith('.mts'), |
There was a problem hiding this comment.
The gate advertises coverage across every fixture under tests/consumer-typecheck/src/, but this only reads direct children of src/. If a fixture is added in a nested folder (a common organization change), its assertions are ignored and the ratchet will report unmet obligations even though valid coverage exists, creating false CI failures.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mechanizes the rule documented in CONTRIBUTING.md after #3480. Every public SuperDoc method or getter must satisfy its computed type obligations via a typed consumer fixture, or be explicitly allowlisted with a reason.
Why this exists
The
SuperDoc.js→SuperDoc.tsmigration shippedsearch(text: string), a narrowed version of the previousstring | RegExpcontract. Every existing gate passed -check:types,check:public:superdoc, the consumer matrix, the deep-type audit, 1054 tests. The bot caught it because no consumer fixture assertedParameters<SuperDoc['search']>. The miss class: an unguarded public method can ship with wrong-but-explicit types.This gate closes that class. It does not guarantee the assertion is correct (a typed-but-wrong assertion would still pass); it guarantees a reviewer was asked to write one - per obligation. The gate is obligation-based, not mention-based: for each public member, the AST computes which assertions are meaningful, and the fixture must satisfy each one.
Obligation model (per member, from the AST)
parametersreturnsreturnscall(otherwise renaming would slip past silently)Satisfaction patterns
tests/consumer-typecheck/src/*.tsparametersParameters<SuperDoc['name']>returns(method)ReturnType<SuperDoc['name']>returns(getter)SuperDoc['name'](indexed access) ortypeof sd.namecallsd.name(orsuperdoc.name(Call sites do NOT satisfy
parametersorreturnson their own. This is the central distinction from a mention-based ratchet - and the reason v1 of this gate didn't catchsearch. A call sitesd.search('hello')was present;Parameters<SuperDoc['search']>was not.Failure modes (all verified by simulation)
Parameters<SuperDoc['search']>fromsearch-match.ts→ FAIL with+ search:parameters. Proves the gate catches the bug class it was built for.--writerefresh hint.broadcastReady: missing or empty reason.noSuchMethod: not a public member of SuperDoc (typo or stale entry).Files
tests/consumer-typecheck/check-public-method-coverage.mjs- the gate (~300 lines).tests/consumer-typecheck/public-method-coverage-allowlist.cjs- escape hatch. 8 entries today (6broadcast*lifecycle relays +setActiveEditor+onContentError). Each key validated against the AST; each value must be a non-empty string.tests/consumer-typecheck/public-method-coverage-debt-snapshot.json- 59 unmet obligations across 36 non-allowlisted members. Future PRs drain via fixtures +--write.scripts/check-public-contract.mjs- new stage 4public-method-coveragebetweenjsdoc-ratchetandbuild. Stage count: 9 → 10. Docstring + stage blurb use obligation language consistently.AGENTS.md,packages/superdoc/scripts/README.md- refreshed stage count + descriptions to include the new stage.End-to-end
OK 75 obligations across 36 members; 59 tracked as known debt; ratchet snapshot in sync.pnpm check:public:superdoc --skip-build→ PASS, 9 ran / 1 skipped, ~120s; new stage runs as[4/10].Scope kept tight
SuperDocEventMapevents SuperDoc emits directly. Separate planned PR (different shape, ordered cheap → expensive:ready,editorCreate,editorBeforeCreatefirst).AssertEqual<>enforcement. The current rule is "an assertion exists per obligation." Tightening to "the assertion matches the source signature exactly" requires a TS program (not just AST parsing), is significantly more expensive, and can be a future tightening after the debt drains.