feat(scripts): js contract-owner audit, report-only (SD-673)#3529
Merged
Conversation
Surveys which .js files own emitted public .d.ts files across the superdoc and @superdoc/super-editor packages, classified against the existing check-jsdoc.cjs tracking state. Report-only: always exits 0; informational output is the survey input for follow-up types-only extraction and @ts-check adoption. Mechanism: - For each of the two packages, walks every typed exports entry, follows relative + self-package edges through the emitted .d.ts forest, and collects every reachable declaration. - Resolves each reachable .d.ts to its source via the companion .d.ts.map sourcemap (sources[0] is the in-repo owner). - For each .js source, classifies as one of: ts-owned (not .js at all, just included for surface area), checked-files (hand-curated // @ts-check gate), has-ts-check (informational), allowlisted, tracked-debt, or UNACCOUNTED (public-surface JS source with no @ts-check directive and no tracking entry). Why two walks: check-jsdoc.cjs already walks from superdoc's entry points and reaches into super-editor JS via implementation imports. But super-editor publishes its own public exports (./types, ./ui, ./markdown, ./presentation-editor, etc.). Files reached only via super-editor's own publishings show up as unaccounted here even when they don't show up in the superdoc-side ratchet. Numbers on origin/main at the time of this PR: superdoc walk (14 typed exports, 483 reachable .d.ts, 479 owners): ts-owned 344 checked-files 4 has-ts-check 11 allowlisted 0 tracked-debt 31 unaccounted 89 @superdoc/super-editor walk (10 typed exports, 364 reachable .d.ts, 364 owners): ts-owned 101 checked-files 3 has-ts-check 31 allowlisted 0 tracked-debt 99 unaccounted 130 The two unaccounted lists overlap (e.g. InputRule.js appears in both); the per-walk numbers above are not deduplicated. tracked-debt 31 + 99 = 130 vs check-jsdoc.cjs's 102 'tracked as known debt' is explained by the same overlap. What this PR does NOT do: - Gate on growth. Promotion to a no-growth ratchet is a separate follow-up once UNACCOUNTED stabilizes at zero per package. - Move any JS files to TypeScript. The script is survey-only; conversion / types-only extraction is targeted manually using this report as input. - Touch check-jsdoc.cjs or the debt snapshot. Both stay the source of truth for their respective domain; this script reads them without mutating. Wired as a wrapper stage (js-contract-owners-report) after root-classification-closure, alongside the other after-build stages. Always exits 0. Adds ~0s to wrapper duration (script is fast: sourcemap reads only, no tsc). Verified: - node packages/superdoc/scripts/report-js-contract-owners.cjs -> OK - pnpm check:public:superdoc --skip-build -> PASS (12 ran, 1 skipped, 130.8s)
…t from wrapper
Three review-driven tightening changes to the JS contract-owner audit:
1. Extract CHECKED_FILES + REACHABILITY_EXEMPT_CHECKED_FILES into a
shared module (packages/superdoc/scripts/jsdoc-checked-files.cjs)
consumed by both check-jsdoc.cjs and report-js-contract-owners.cjs.
Replaces the prior inlined KNOWN_CHECKED_FILES_REFERENCE in the
audit script, which created a drift trap reviewers would have had
to police manually.
2. Drop the audit from the check:public:superdoc wrapper chain. The
audit is report-only (always exits 0) and does not enforce
anything; wiring it as a wrapper stage adds CI noise without
adding a contract. Becomes a standalone command:
node packages/superdoc/scripts/report-js-contract-owners.cjs
When the audit is later promoted to a strict no-growth ratchet,
THAT version earns a wrapper-stage entry.
3. README updated to reflect the standalone runtime + the shared
config source.
Verified:
- node packages/superdoc/scripts/report-js-contract-owners.cjs
-> OK (89 unaccounted on superdoc walk; 130 on super-editor walk;
identical to pre-refactor output)
- node packages/superdoc/scripts/check-jsdoc.cjs
-> OK (128 / 4+2 / 26 / 0 / 102, identical to pre-refactor output)
- pnpm check:public:superdoc --skip-build still passes (no wrapper
stage added or removed since the audit was never in the previous
commit's wrapper at run time)
Two review-driven fixes to the JS contract-owner audit: 1. Missing dist now exits 1, not 0. Previously the script printed SKIP and exited 0 when either package's typed export targets were missing on disk. "Report-only" should mean "findings (UNACCOUNTED count) never fail" — not "missing inputs still succeed." A broken input pipeline is distinguishable from a clean run now, and the script tells the user what to run to fix it (pnpm build or pnpm run type-check). 2. check-jsdoc.cjs header doc updated to point contributors at the shared jsdoc-checked-files.cjs module (the new source of truth) instead of "append below." Trivial doc nit, but exactly the kind of stale instruction that misleads new contributors after a refactor. Verified: - happy path: node report-js-contract-owners.cjs -> exit 0 - structural failure (super-editor dist/src removed): -> exit 1 - check-jsdoc.cjs still passes unchanged
The README row still said 'Always exits 0' after the previous commit flipped the script to exit 1 on structural failures. Now matches the actual behavior: findings (UNACCOUNTED count) exit 0; missing dist or unreadable package inputs exit 1.
Contributor
![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: a212720f75
ℹ️ 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".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The remediation message suggested 'pnpm build (or pnpm run type-check
to emit declarations only).' That alternative is wrong:
- packages/superdoc/tsconfig.types.json sets outDir to dist-types,
not dist.
- The audit walks packages/superdoc/dist (the consumer-visible
tree via package.json exports).
- So pnpm run type-check alone leaves packages/superdoc/dist empty
and the audit keeps failing with the same structural error.
Now recommends pnpm build only, in three places:
- script header doc
- per-package missing-target error
- final structural-failure hint
README row updated to match.
The script behavior is unchanged; only the user-facing remediation
hint was misleading.
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.
Surveys which
.jsfiles own emitted public.d.tsfiles across bothsuperdocand@superdoc/super-editorpackages, classified against the existingcheck-jsdoc.cjstracking state. Standalone report; not wired intocheck:public:superdoc. Run on demand to seed follow-up types-only extraction or@ts-checkadoption.How to run
Requires both package dists to exist on disk. Run after
pnpm buildso the audit has something to walk. (pnpm run type-checkis not a substitute: it writes superdoc declarations todist-types/perpackages/superdoc/tsconfig.types.json, notdist/.)Exit semantics
How it works
exportsentry, follows relative + self-package edges through the emitted.d.tsforest, collects every reachable declaration..d.tsto its source via the companion.d.ts.mapsourcemap (sources[0]is the in-repo owner)..jssource, classifies as one of:ts-owned,checked-files(hand-curated// @ts-checkgate),has-ts-check(informational),allowlisted,tracked-debt, orunaccounted.Shared source of truth
CHECKED_FILES+REACHABILITY_EXEMPT_CHECKED_FILESmoved out ofcheck-jsdoc.cjsinto a newpackages/superdoc/scripts/jsdoc-checked-files.cjs. Bothcheck-jsdoc.cjs(gate) andreport-js-contract-owners.cjs(audit)require()it. Zero drift between the two; edits go in one place.Why two walks
check-jsdoc.cjswalks fromsuperdoc's entry points and reaches super-editor JS via implementation imports. But@superdoc/super-editorpublishes its own public exports (./types,./ui,./markdown,./presentation-editor, etc.). Files reached only via super-editor's own publishings show up as unaccounted here even when they don't show up in the superdoc-side ratchet.Inventory on origin/main at PR time
superdocwalk (14 typed exports, 483 reachable.d.ts, 479 distinct owners):Also: 4 no-owner declarations (reachable
.d.tswhose source couldn't be resolved — allno-sourcemap; small blind spot worth flagging).@superdoc/super-editorwalk (10 typed exports, 364 reachable.d.ts, 364 distinct owners):The two unaccounted lists overlap (e.g.
InputRule.jsappears in both); per-walk numbers are not deduplicated.tracked-debt31 + 99 = 130 vscheck-jsdoc.cjs's 102 "tracked as known debt" is explained by the same overlap — cross-validation that the classifier is reading the shared snapshot consistently.What this PR does NOT do
Sequencing note
This PR and #3528 both touch
packages/superdoc/scripts/README.md(different rows in the same script-catalog table). The one that lands second needs a one-line rebase.Verified:
node packages/superdoc/scripts/report-js-contract-owners.cjs→ exit 0 (happy path, numbers above)dist/srcremoved) → exit 1 with a clear "runpnpm build" messagenode packages/superdoc/scripts/check-jsdoc.cjs→ OK (128 / 4+2 / 26 / 0 / 102, unchanged after shared-module refactor)pnpm check:public:superdoc --skip-build→ PASS (11 ran, 1 skipped, 135.0s)