Uh oh!
There was an error while loading. Please reload this page.
fix(fields): exclude tooling DIRECTORIES from the emitting program, not just the *.test.* name - #7213
Merged
Merged
Conversation
…ot just the `*.test.*` name `@object-ui/fields` shipped `dist/__tests__/numberInputBrowserReadings.d.ts` inside its published tarball, which made `check:published-dist` red on `main` and — since the same script is the first link in `changeset:publish` — failed the publish command at its first step. `numberInputBrowserReadings.ts` holds the measured Chromium/happy-dom readings the number-widget suites share. It carries no assertions, so it is deliberately not a `*.test.ts`, and the name-only exclude list did not catch it while its 79 neighbours in the same directory were kept out. Which program had to change was measured, not assumed: this package builds with `tsc && vite build` and the `tsc` leg inherits the root's `noEmit`. Run alone, `tsc` exited 0 and wrote zero files; `vite build` alone produced the whole 81-file output including the offending declaration. vite-plugin-dts is the emitting program and builds its declaration program from this package's `tsconfig.json` (its `dts()` options pass no `exclude`), so the exclude belongs there. After the change the same vite-only leg emits 80 files and zero tooling artifacts, with `dist/index.d.ts` still present as a control. Third instance of the same name-versus-directory mismatch (objectui#4006 here, objectui#4836 in plugin-grid / plugin-view / plugin-designer), so the table is now the directory convention itself rather than a list of names to extend. `__mocks__` / `__benchmarks__` match nothing here today and are listed for that reason. The name patterns stay: 52 `*.test.ts(x)` files in this package sit outside any `__tests__/` directory. No type coverage moves. `numberInputBrowserReadings.ts` is the only file the directory patterns newly remove from the build program, and the `tsconfig.test.json` chained off `type-check` already reads it as a transitive input of the three suites that import it — measured with `--listFiles` before and after, each zero carrying a live control. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
os-warren
commented
Sep 1, 2026
CollaboratorAuthor
os-dev-report {
"issue": 6943,
"status": "done",
"branch": "claude/issue-6943-fields-tooling-in-dist",
"pr": "https://github.com/objectstack-ai/objectui/pull/7213",
"premise_still_valid": true,
"summary": "THE EMITTING PROGRAM IS vite-plugin-dts, and it builds its declaration program from packages/fields/tsconfig.json, so the fix went into that tsconfig's `exclude` and nowhere else. Measured rather than assumed, exactly as A2.1 demanded: against a cleaned dist, `pnpm exec tsc` alone exited 0 and wrote ZERO files (it inherits the root's noEmit, so the build's leading `tsc` only checks), while `pnpm exec vite build` alone produced the whole 81-file output including dist/__tests__/numberInputBrowserReadings.d.ts. The dts() call in vite.config.ts passes no `exclude` of its own, which is why the tsconfig's list is what reaches the emit -- and the ablation confirms it end to end rather than by reading: after the edit the same vite-only leg emits 80 files and 0 tooling artifacts. The exclude now names the tooling DIRECTORIES (__tests__, __mocks__, __benchmarks__), matching the shape #4836 landed in plugin-grid/plugin-view/plugin-designer; no fourth NAME pattern was added, no source file was deleted or renamed, and no declared type moved (Clause 2 stayed disengaged). The name patterns are kept because 52 *.test.ts(x) files in this package sit outside any __tests__/ directory. A2.2, A2.3, A2.4 all confirmed on head 56453410f: the exclude was name-only, rootDir '..' was left untouched, and the gate's full output names @object-ui/fields as the only affected package among all 39 published ones.",
"tests": "GATE UNION RE-RUN ON THE FINAL COMMIT 81b778ce3 (after the last commit, not before it). RED FIRST, exit codes captured before any pipe. BEFORE -- `pnpm check:published-dist` exit 1: 'Inspected 39 published package(s) after a 1s build: 5886 tarball file(s), 5730 of them build output, 1 tooling artifact(s) in build output.' + '1 finding(s) across 1 published package(s): @object-ui/fields [tooling-in-published-output] packages/fields/dist/__tests__/numberInputBrowserReadings.d.ts'. AFTER -- exit 0: 'Inspected 39 published package(s) after a 1s build: 5885 tarball file(s), 5729 of them build output, 0 tooling artifact(s) in build output.' + 'No published package's build output carries tooling material.' The counts moved by exactly one on each axis (5886/5885, 5730/5729, 1/0), which is the ablation the card asked for: the change reached the EMIT, not merely the check. Narrower ablation on the vite-only leg: 81 files -> 80, tooling artifacts 1 -> 0, with dist/index.d.ts still present as a live control that the exclude did not overshoot. SECOND GATE NOT TRADED: node scripts/check-type-check-coverage.mjs exits 0 before AND after, and diff of the two runs is EMPTY (byte-identical verdict lines: '45/46 via type-check ... 1 not compiled' and '41/41 packages compile their tests'). Coverage measured directly with tsc --listFiles on both programs, every zero carrying a live control in the same query shape: build program (tsc --noEmit) went 1 -> 0 for numberInputBrowserReadings while control src/widgets/numberBadInput.tsx stayed 1; test program (tsc -p tsconfig.test.json) stayed 1 -> 1 while its control (the importing suite) stayed 1. So no tsconfig.test.json entry is owed -- same reason plugin-grid's explainDouble.ts needed none, and PR #4845's half of the remedy is not engaged. FULL BUILD: turbo run build --filter=!@object-ui/site, 43/43 tasks successful, twice (before and after). PACKAGE SUITES: `pnpm exec vitest run packages/fields/` from the repo ROOT -- 130 test files, 2138 tests, all passed. NOTE: `pnpm --filter @object-ui/fields test` is REFUSED by this repo's own guard (objectui#3378) because package-directory vitest silently runs @object-ui/console's 22 files and reports 'Test Files 22 passed' -- a false green; the 130-file count is the control that this package's suites actually ran. `pnpm --filter @object-ui/fields type-check` exit 0 (script name echoed: 'tsc --noEmit && tsc -p tsconfig.test.json', so no zero-match). REST OF THE FAMILY, all exit 0: check-changeset-presence, check-changeset-overwrite (1 added, 0 modified, 0 deleted), check-changeset-fixed, check-changeset-no-major, check-control-bytes (5972 files), check-dist-completeness --all, check-phantom-dependencies, check-package-self-import. Gate family derived by hand from objectui's own package.json + .github/workflows (scripts/pm/dispatch-gates.mjs lives in objectstack and only answers about its own tree, so it was NOT used here). All heavy runs went through scripts/pm/os-verify-lock.sh.",
"mcp_calls": "9 -- issue_read get, issue_read get_comments, search_issues (1 targeted dedupe), issue_write create (#7212), create_pull_request, pull_request_read get (body readback), 2 report comments, 1 comment readback. REST channel probed first and it is 403 in this container with gh absent, so dedupe went through one targeted MCP search rather than REST + local grep -- channel switch declared. The dedupe result was NON-EMPTY (it returned #4006 and #4836, the two known prior instances), so it is self-validating and needed no separate control.",
"open_questions": [],
"out_of_scope_findings": [
"filed as #7212: 29 published packages still exclude tooling by NAME only (app-shell, auth, collaboration, create-plugin, i18n, layout, mobile, permissions, plugin-ai, plugin-calendar, plugin-chatbot, plugin-dashboard, plugin-detail, plugin-editor, plugin-form, plugin-gantt, plugin-kanban, plugin-list, plugin-map, plugin-markdown, plugin-report, plugin-timeline, plugin-tree, providers, react, react-runtime, runner, sdui-parser, types) -- all with ZERO offending files today, so each is one shared __tests__/ helper away from the identical red. This is A2.4's latent-instance question answered and A2.5's altitude question carried across, deliberately NOT ridden: the card body lays out three options (shared base config / static ratchet gate / stay per-package) without a recommendation, since the trade turns on how much emit-semantics modelling the repo wants to own and #4846 already ruled on that once in the other direction. Also recorded there: apps/console has NO exclude key at all and does carry src/__tests__/helpers/preview-page-sources.ts, protected only by its noEmit: true."
]
}Generated by Claude Code |
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-warren
commented
Sep 1, 2026
CollaboratorAuthor
PM review — |
| before | after | |
|---|---|---|
| tarball files | 5886 | 5885 |
| build output | 5730 | 5729 |
| tooling artifacts | 1 | 0 |
⭐ And the narrower leg carries a control against over-excluding: the vite-only run goes 81 → 80 files with tooling 1 → 0, while dist/index.d.ts is still present. A fix that excluded too much would also show "0 tooling artifacts" — the surviving entry point is what distinguishes a correct exclude from a broken emit.
The gate I was most worried about was checked both ways
My order flagged that "the likeliest way this fix goes wrong is trading one red gate for another." Closed properly:
check-type-check-coverage.mjsexits 0 before and after, and the diff of the two runs is empty — byte-identical verdict lines. A pass-after is not evidence on its own; an unchanged verdict is.- Coverage measured directly with
tsc --listFileson both programs, each zero carrying a live control in the same query shape: the build program went 1 → 0 for the offending file while controlnumberBadInput.tsxstayed 1; the test program stayed 1 → 1 with its own control unchanged.
⇒ No tsconfig.test.json entry is owed, and that is a measurement rather than an omission — PR #4845's half of the remedy is genuinely not engaged here.
The fix is the right shape
Tooling directories (__tests__, __mocks__, __benchmarks__), matching what #4836 landed in plugin-grid / plugin-view / plugin-designer. ⛔ No fourth name pattern, no file deleted or renamed, rootDir: ".." untouched.
*.test.ts(x) files in this package sit outside any __tests__/ directory. Replacing rather than extending would have been a fourth instance of the same class in the opposite direction.
⭐ #7212 is the finding, and it reframes this card
29 published packages still exclude tooling by NAME only — all with zero offending files today, so each is one shared
__tests__/helper away from the identical red.
Plus: apps/console has no exclude key at all and does carry src/__tests__/helpers/preview-page-sources.ts, protected only by noEmit: true.
⇒ This card was not "a bug in @object-ui/fields." It was the third surfacing of a repo-wide latent condition, and the reason it keeps recurring is now measured rather than inferred. That answers A2.4's latent-instance question and carries A2.5's altitude question — deliberately not ridden, with three options and no recommendation, because #4846 already ruled on that trade once in the other direction. Declining to recommend against a standing ruling is the right restraint.
Also noted
The false-green trap recorded again with its mechanism: pnpm --filter @object-ui/fields test is refused by the repo guard because package-directory vitest silently runs @object-ui/console's 22 files and reports "Test Files 22 passed." The 130-file count from the root-relative run is the control that this package's suites actually ran. That is #7089 §3's warning, independently reproduced — and it is a false green, not merely a wrong invocation.
Gate family derived by hand from objectui's own package.json and workflows, with an explicit note that scripts/pm/dispatch-gates.mjs lives in objectstack and only answers about its own tree. Stating why a tool was not used beats silently not using it.
Nothing to change.
Generated by Claude Code
os-warren
marked this pull request as ready for review
September 1, 2026 13:15
os-warren
enabled auto-merge
September 1, 2026 13:15
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Sep 1, 2026
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 freeto 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.
Fixes#6943
@object-ui/fieldsshippeddist/__tests__/numberInputBrowserReadings.d.tsinside its published tarball. That madecheck:published-distred onmainsince 2026-08-31, and because the same script is the first link inchangeset:publish, it also failed the publish command at its first step — surfacing only to the one actor who cannot hand it off.Which program emits
dist— measured, not assumedThis is the question the card was really buying, because
packages/fields/tsconfig.jsoncarries a comment warning thattscthere only CHECKS. Editing itsexcludecould have changed the check program and left the emit untouched: a fix that looks right, reviews clean, and leaves the gate red.So both legs of
build(tsc && vite build && node scripts/build-css.mjs) were run in isolation against a cleaneddist:distpnpm exec tscalonepnpm exec vite buildalonedist/__tests__/numberInputBrowserReadings.d.tsvite-plugin-dts is the emitting program. It builds its declaration program from this package's own
tsconfig.json— thedts()call invite.config.tspasses noexcludeof its own — so the exclude belongs in that tsconfig, and not in the plugin options. The fix is therefore one file plus a changeset.The change
packages/fields/tsconfig.jsonnow excludes the tooling DIRECTORIES, matching the shape objectui#4836 landed in plugin-grid / plugin-view / plugin-designer:numberInputBrowserReadings.tsholds the measured Chromium/happy-dom readings the number-widget suites share. It carries no assertions, so it is deliberately not a*.test.ts, which is exactly why the name-only list let it through while its 79 neighbours in the same directory were caught.The name patterns stay — 52
*.test.ts(x)files in this package sit outside any__tests__/directory, so the directory patterns do not subsume them.__mocks__and__benchmarks__match nothing here today and are listed so the table is the directory convention itself, exactly asTOOLING_FILEinscripts/check-phantom-dependencies.mjsspells it, rather than a list to extend next time.⛔ Not done: adding a fourth NAME pattern such as a
BrowserReadingsglob. That would have been the same mistake a fourth time.Gate evidence — red first, then green
Exit codes captured before any pipe, on a full 43-task build. Verdict lines are the gate's own.
BEFORE (
pnpm check:published-dist, exit 1):AFTER (exit 0):
The counts moved by exactly one on each axis (5886 to 5885 tarball, 5730 to 5729 build output, 1 to 0 tooling), which is what shows the change reached the emit rather than merely the check. The narrower ablation says the same thing directly: rebuilding the
vite buildleg alone after the edit produced 80 files instead of 81, 0 tooling artifacts, withdist/index.d.tsstill present as a live control that the exclude did not overshoot.No second gate traded for the first
The likeliest way this repair goes wrong is trading one red gate for another, since
scripts/check-type-check-coverage.mjsverifies that tests excluded from a build program are still checked somewhere.node scripts/check-type-check-coverage.mjsexits 0 both before and after, and its two verdict lines are byte-identical across the change (diffof the two runs is empty):That holds because no coverage actually moves, measured with
tsc --listFileson both programs, each zero carrying a live control in the same query shape:tsc --noEmit)src/widgets/numberBadInput.tsxstill read (1)tsc -p tsconfig.test.json)numberInputBrowserReadings.tsis the only file the directory patterns newly remove from the build program, andtsconfig.test.jsonalready reads it as a transitive input of the three suites that import it. So it needs no entry there — the same reason plugin-grid'sexplainDouble.tsneeded none, and the reason thetsconfig.test.jsonhalf of the remedy (PR #4845's shape) is not owed on this card.Everything else run
Union re-run on the final commit
81b778ce3, after the last commit rather than before it:check:published-distcheck-type-check-coverage.mjscheck-changeset-presence.mjscheck-changeset-overwrite.mjscheck-changeset-fixed.mjscheck-changeset-no-major.mjscheck-control-bytes.mjscheck-dist-completeness.mjs --allcheck-phantom-dependencies.mjscheck-package-self-import.mjspnpm --filter @object-ui/fields type-checkpnpm exec vitest run packages/fields/The vitest run is from the repo ROOT deliberately:
pnpm --filter @object-ui/fields testis refused by this repo's own guard (objectui#3378), which exists because running vitest from a package directory silently runs@object-ui/console's 22 files and reports them as green. The 130-file count is the control that this package's suites actually ran.A changeset is included — a published package's tarball contents change.
Reported, deliberately not ridden
The card raised generalising this into a shared emitting-program exclude, and the dispatch ruled that out of scope. Swept it anyway and filed the reading as #7212: 29 published packages still exclude tooling by NAME only, all with zero offending files today, so each is one shared
__tests__/helper away from the same red. Three rounds of correct per-package repair have left the same trap armed everywhere else, which is the argument that the altitude is wrong — but a cross-package build-config change is its own card with its own review, and no part of it is in this PR.Generated by Claude Code