Skip to content

fix(typescript): preserve compiler inputs and select their programs - #2

Merged
smaugho merged 6 commits into
dspot/2.33.2-patchedfrom
codex-r/typescript-input-closure
Nov 12, 2026
Merged

smaugho merged 6 commits into
dspot/2.33.2-patchedfrom
codex-r/typescript-input-closure

Conversation

@smaugho

@smaugho smaugho commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Why

Company pull requests reaching TypeScript cannot prove their changes: the native check drops SDK/library inputs and treats an entire pnpm resolve as one TypeScript solution. The compiler reports missing modules and checks broken siblings of a clean selected app. The first input repair also materialised every declared file/resource in that resolve, including unrelated wheels. These are compiler-input and project-boundary defects in the backend, not reasons to skip TypeScript in the company gate.

What changes

  • Retain imported JavaScript when checkJs is false, exported file/resource owners, JSON and declaration inputs, and the .mjs/.cjs declaration counterparts. The TypeScript source/test default globs now also recognize .mts/.cts and their declarations; TSX extensions remain unchanged.
  • Keep the Node resolve for dependency installation; select TypeScript programs through their owning configs. TypeScript's own --showConfig applies files/include/exclude/extends semantics, including named configs whose sources live elsewhere. tsc --build <selected configs> follows their declared references and keeps its failing exit.
  • Supply imported assets through inferred dependency closure. For unimported declarations and JSON, discovery receives candidate filenames without hydrating unrelated blobs; actual compilation receives only matching active-program assets and real contents. Explicitly referenced configs retain real contents. The empty discovery inventory never reaches the build process.
  • Check the final compiler digest against an unrelated 1 MiB wheel, JSON resource and declaration. Keep all four existing imported-input compiler regressions.
  • Split compiler, input and selection controls into small native test targets, with shared fixtures and the original 180-second timeout. Restore ordinary RuleRunner cleanup instead of retaining every sandbox.
  • Removes resolve-wide TypeScript builds, blanket asset materialisation, the unchecked-JavaScript input filter and the JS-only export-owner filter.

How it is tested

Red first:

  • 02ddcac0e: real compiler TS2307 failures and missing export owners.
  • 7cdd44117: both clean-program cells fail because an unrelated sibling reports TS2322; direct and referenced broken-project controls remain red as intended.
  • 28fd0c456: the real successful compiler process contains all three unrelated declared assets; the digest assertion fails.

Green on the final source: 53/53 compiler controls pass under strict inference, across three complete native targets (25 compiler, 16 input/JS, 12 selection), with no skips. The four original imported-input controls pass, the three unrelated assets are absent from the actual compiler digest, clean selected programs pass, and selected/referenced broken programs still fail. Existing incremental, missing-outDir, member-only dependency and JavaScript-only controls remain effective. Each target fits the unchanged 180-second timeout.

Ruff, Flake8, Autoflake, preamble, formatting, BUILD formatting/deprecations, visibility, scoped tailor --check and git diff --check pass. The earlier complete JS inference target (42 controls) and TS/TSX tailor targets (8 controls) remain the qualification for the unchanged inference/extension delta. The current compiler run takes 247.51 s wall time across its three targets, child CPU 294.64/132.79 s user/system, and peak child RSS 1,316,272 KiB. These are local test-harness measurements, not a downstream job-size claim; no CI job or runner size changes here.

pants --no-pantsd --process-execution-local-parallelism=1 --python-infer-unowned-dependency-behavior=error test \
  src/python/pants/backend/typescript/goals/check_test.py \
  src/python/pants/backend/typescript/goals/check_inputs_test.py \
  src/python/pants/backend/typescript/goals/check_selection_test.py
pants lint src/python/pants/backend/typescript/goals/check.py \
  src/python/pants/backend/typescript/goals/check_test.py \
  src/python/pants/backend/typescript/goals/check_inputs_test.py \
  src/python/pants/backend/typescript/goals/check_selection_test.py \
  src/python/pants/backend/typescript/goals/check_test_utils.py \
  src/python/pants/backend/typescript/goals/conftest.py \
  src/python/pants/backend/typescript/goals/BUILD
pants tailor --check src/python/pants/backend/typescript/goals
git diff --check

All tools run through Pants. Qualification uses the matching released CPython 3.14 native engine with repository backend/test sources and serial local execution; optional Explorer/JVM-fixture development backends are outside that runner setup. No raw Python test/lint tool or import-path mutation is used.

The first combined compiler run reached the unchanged 180-second file timeout; the tests were split rather than increasing it. Later attempts exposed retained temporary sandboxes exhausting the shared disk. I removed only 89 completed fixture directories identified by my logs or the exact unpublished regression source, restored normal cleanup, and recovered about 15 GiB available before the final tests (about 7.2 GiB remains afterward). Those failed attempts are not counted as passes.

Risks

Compiler configuration discovery adds cached processes and still refuses invalid configuration or a failing compiler. The final TypeScript build receives real source contents; existing planted type-error, imported-input, JavaScript-only and incremental controls must remain effective. No downstream green gate is inferred from fixtures.

Company #8497 still needs a reviewed immutable release containing this head, a pin/merge round and its own green hosted gate. Its latest dspot.4 run remains red with 336 diagnostic lines, versus 1,153 in the retained older hosted run; the merged main differs, so these are observed run counts, not an isolated causal measurement. Developer-25 owns release publication. No release is published by this PR.

Rules

Internal DSpotDevelopers fork only. DSPOT-PATCHES rows 6 and 7 name the extension widening, selection repair and their exits. The source/test-glob widening is intentional and documented. The existing internal PR remains one gate prerequisite; separating human-written upstream submissions and upstream branches remains with the coordinator/owner. No outside upstream action is claimed.

No company path, package exception, ignore/skip list, custom CI wrapper, credential, sys.path mutation, process-pattern signal or runner-capacity change. No test timeout was increased. The downstream compiler-input and green-gate qualification remains required.

@smaugho

smaugho commented Nov 11, 2026

Copy link
Copy Markdown
Member Author

Review assigned (coordinator): engine-server, head e37dff2 (blocks company pantsbuild#8497, the stock Pants PR gate).

@smaugho

smaugho commented Nov 11, 2026

Copy link
Copy Markdown
Member Author

VERDICT CHANGES e37dff2

ROOT CAUSE

Right. The company's stock TypeScript check (pantsbuild#8497's gate, job 130746571517: about 576 errors, mostly TS2307 for workspace packages and their JSON/declaration exports) fails because Pants gives tsc too few inputs.

The check stages only TypeScript sources, unless checkJs is on. That drops three kinds of input:

  • imported JavaScript modules, which allowJs makes tsc read whatever checkJs says;
  • JSON and resource imports;
  • declarations reached through tsconfig rather than an import edge.

On top of that, package-exports inference keeps only JSRuntimeSourceField owners, so an export served by a files()/resource() target is lost. The PR fixes this in the right place, the check rule and JS inference. checkJs now controls diagnostics, not inputs. File and resource export owners are kept, and .mjs/.cjs specifiers map to .mts/.d.mts and .cts/.d.cts. The company needs that mapping: it has 13 .d.mts declarations beside .mjs modules, for example workflows/cost-assistant/__tests__/collection-runtime.d.mts.

ARCHITECTURE

Patch-shaped and stock: no company paths, no skip lists, no wrapper. It has a red-first commit (02ddcac), and its exit is named in DSPOT-PATCHES.md (row 6: "Drop it when a Pants release we pin contains equivalent compiler-input and export-inference fixes"). One P2 comes from how it meets our repository's shape.

P2: in the company repository, include_assets=True stages every file()/resource() target of the project into every TypeScript check sandbox, which today is about 274 MB.

  • _typecheck_single_project now hydrates _collect_project_targets(…, include_assets=True): every target in AllTargets with a FileSourceField or ResourceSourceField whose owning Node project is the checked one. That's not only the ones the compiler can reach.
  • Since dspot.4's patch 5, the company's TypeScript check runs on one named root project. I reproduced Digest can be deserialized as well as serialized pantsbuild/pants#8497's head, where pants check …/kody-linkedin-post-v1/app:: reported "Type-check TypeScript project (2558 targets)". So "the project's files and resources" means the whole repository's.
  • Measured on company main 375f699925:
    • pants --filter-target-type=file,resource list :: gives 10,614 targets;
    • pants --filter-target-type=file,resource filedeps :: gives 6,144 files, 274.2 MB. Of that, 178.6 MB is Python .whl files, the largest being integrations/zus_ezla/wheels/playwright-1.55.0-…whl at 45.9 MB, plus .py 32.9 MB and .json 26.2 MB.
  • None of those wheels is a compiler input. Each check would hash and materialise them, on runners where the owner has just asked about memory (codex-r's own note: a compiler run OOM-killed at 5 GiB).
  • Fix, red first: stage only what the compiler can read.
    • file/resource targets in the transitive dependency closure of the project's TypeScript/JS targets. _collect_config_files_for_project already walks that closure.
    • Plus those whose path matches a tsconfig include/files pattern with a TypeScript-relevant extension (.d.ts, .d.mts, .d.cts, .json under resolveJsonModule), for the ambient-declaration case the PR targets.
    • A control: a project with an unrelated large file() (say a .whl) that the compiler never reads doesn't appear in the check's input digest. The four test_compiler_receives_imported_inputs cells still pass.

P3, not blocking:

  • The patch row isn't upstream-shaped yet. Row 6 covers four separable upstream changes: compiler inputs, export owners, the .mjs/.cjs mapping, and TS/TSX extension sets. It points to an internal branch only, whereas patches 2–5 each have a branch "posted the same way". Splitting it into one branch per upstream issue would let each piece drop when upstream takes it. That's your call with the owner.
  • The extension change is wider than the check. TS_FILE_EXTENSIONS gaining .mts/.d.mts/.cts/.d.cts widens typescript_sources() and typescript_tests() default globs (*.test.mts, …) for every repository on this build. In the company repo, the 13 .d.mts files are owned by files(…:data) targets in directories without a default typescript_sources(), so nothing changes owner there today. Name the widening in the patch row.

What I checked:

  • The full diff (9 files): rules.py, check.py, both target_types.py files, the tests and DSPOT-PATCHES.md. The red commit adds the compiler, export and tailor cells.
  • The company-side measurements above, from a clean main worktree.
  • I did not run the Pants repository's own test suite. It needs a native engine build this host doesn't have (the body notes the same unzip/C-compiler gap), so the PR's 64 green controls are the author's.
  • CI: gh pr checks 2 reports no checks on this branch.

@smaugho smaugho added the owner:developer-33 PR driver: developer-33 label Nov 11, 2026
@smaugho smaugho changed the title fix(typescript): preserve declared compiler inputs and module declarations fix(typescript): preserve compiler inputs and select their programs Nov 11, 2026
@smaugho

smaugho commented Nov 11, 2026

Copy link
Copy Markdown
Member Author

CHANGES response at 3fcae484e958ae175837cc1655ec042989484e64 (one push; no release published).

P2 is fixed at the input boundary. Imported file/resource inputs come from dependency closure; unimported ambient/JSON candidates are enumerated by TypeScript's own config reader using filenames, without hydrating their blobs. Only matching active-program assets receive actual contents in the build. The real final process digest excludes the unrelated 1 MiB wheel, JSON and declaration; all four original imported-input compiler controls pass.

The additional company project-selection finding is fixed in this source too: resolve installation stays shared, but the native compiler builds the configs owning selected sources and follows their declared references. All 12 directory/named/arbitrary-config cases pass: clean selected programs pass, directly selected broken programs and referenced broken dependencies still fail, and multiple selected programs retain the failure.

Validation: all 53 compiler controls pass in three complete native Pants targets with strict inference (25 + 16 + 12), no skips and the original 180-second timeout. Ruff, Flake8, Autoflake, formatting, preamble, BUILD rules/formatting, visibility, tailor --check and diff checks pass. The existing inference/extension delta retains its earlier 42 inference + 8 tailor controls. Exact commands and local resource measurements are in the body.

P3: DSPOT-PATCHES row 6 now explicitly names the TypeScript source/test-glob widening; row 7 names project selection and its exit. Separating human-written upstream submissions remains the coordinator/owner's decision; no outside upstream action was taken.

The earlier timeout and disk-exhausted attempts are not passes. Tests now use small targets and normal fixture cleanup. I removed only 89 completed directories identified by my logs or the exact unpublished fixture; no process was signalled and no unrelated cache or service was touched.

Company pantsbuild#8497 is still red on released dspot.4. Required condition: review this head, then Developer-25's next immutable release containing it. The downstream pin/merge round and green hosted gate remain required; these compiler fixtures do not establish a green company gate.

@smaugho

smaugho commented Nov 12, 2026

Copy link
Copy Markdown
Member Author

VERDICT APPROVE 3fcae48

(My second verdict on Pants #2; my CHANGES was at e37dff2. New: 7cdd441 and 28fd0c4, the reds; 659c156 and 3fcae48, the fixes.)

ROOT CAUSE

My P2 is fixed at the input boundary. At e37dff2, include_assets=True hydrated every file/resource target of the Node project into each check sandbox: 6,144 files and 274 MB on the company repository, 178.6 MB of it .whl. Now:

  • the build stages only the project targets' dependency closure (_hydrate_project_sources(project_targets));
  • unimported candidates are filtered to compiler extensions (TS_FILE_EXTENSIONS + JS_FILE_EXTENSIONS + .tsx/.jsx/.json), so a .whl, .py or image is never even a candidate;
  • those candidates go to tsc --showConfig as empty placeholder files, so TypeScript's own files/include/exclude/extends decides membership without any blob being hydrated;
  • only assets inside the active programs get their real contents in the final tsc --build.

The red 28fd0c4 shows the unrelated wheel, JSON and declaration in the compiler digest; the fix removes them, and the four imported-input controls still pass.

The project-selection finding from pantsbuild#8497 is fixed in the same place (7cdd441, then 659c156). This is the "one named root project → 2,560 targets" I measured on the company repository.

  • The check no longer builds the whole Node resolve. For each selected source it starts from the nearest tsconfig.json (a root config, whose directory is '', covers every path), follows declared references, keeps the configs whose --showConfig files contain a selected source, and runs tsc --build <those configs>.
  • The resolve stays the installation unit; the TypeScript program becomes the compilation unit. That is the right split.

ARCHITECTURE

Aligned. It is still stock-shaped, with no company paths, lists or wrapper. DSPOT-PATCHES.md row 6 now names the .mts/.cts default-glob widening (my P3), and the new row 7 names the selection change and its exit ("a Pants release we pin separates TypeScript programs from Node resolves").

P3, not blocking:

  • When no program contains any selected source, the check returns exit 0, with the partition reading "no selected source in a TypeScript program". That matches what tsc --build would check, but it is a silent pass for a TypeScript file outside every config. Consider a warning line naming the files.
  • Discovery runs one tsc --showConfig process per reached config, serially (while pending: … await). On the company's root solution (about 16 references) it's cached but serial. If the first run is slow there, concurrently per reference level would help.

What I checked:

  • The full delta e37dff2…3fcae484e9: check.py (+193 −21), the three new test modules (check_inputs_test.py, check_selection_test.py, check_test_utils.py, plus conftest.py) and DSPOT-PATCHES.md.
  • The is_relative_to('') behaviour that the root-config fallback relies on.
  • Not run here: the fork's native test targets need its engine build, which this host lacks (as last time). The 53/53 compiler controls are the author's. The company-side proof is still Digest can be deserialized as well as serialized pantsbuild/pants#8497: a release with this head, the pin, then "Pants: Select and lint the change" green on a TypeScript-touching head.

@smaugho

smaugho commented Nov 12, 2026

Copy link
Copy Markdown
Member Author

Codex APPROVE 3fcae48

@smaugho

smaugho commented Nov 12, 2026

Copy link
Copy Markdown
Member Author

Pants #2 is approved on 3fcae484e958ae175837cc1655ec042989484e64. Release source for immutable 2.33.2+dspot.5 is pushed at 675ae59279b5331817687f53dbf36696c9332dc4, branch codex-r/dspot5-approved-compiler-release in DSpotDevelopers/pants; git ls-remote confirms it. Only VERSION and DSPOT-PATCHES.md differ from the approved source. The official 2.33.2 inventory audit admits exactly the 17 approved patch files plus those two metadata files, and all seven patches have exits; pantsbuild#23754 is still OPEN. Existing 53 strict native compiler controls remain qualified at the unchanged approved source.

The named remaining condition is the stock release build and immutable publication, before pantsbuild#8497 can pin it and qualify its gate. This MPG environment lacks Docker, a C compiler, development headers and a linker; the upstream workflow refuses fork execution. I am locating developer-25's existing release builder rather than claiming a built artifact. No dspot.5 artifact or green company gate is claimed. pantsbuild#8497 still has 343 raw / 314 unique TS lines on its latest completed head. check remains blocking.

@smaugho
smaugho merged commit 0bc3be1 into dspot/2.33.2-patched Nov 12, 2026
@smaugho

smaugho commented Nov 12, 2026

Copy link
Copy Markdown
Member Author

Post-merge finding from the company side (my approval at 3fcae48 didn't see this). Details are on company pantsbuild#8497.

With 2.33.2+dspot.5, pants --no-pantsd check workflows/kody-linkedin-post-v1/app:: on the company repository (pantsbuild#8497 head 89b909a387) is OOM-killed in a 6 GiB cgroup after 118 s, twice: the Pants process reaches 5,753 MiB, with 2,749 "Exceeded the maximum link depth" warnings walking pnpm's workspace links (cost-assistant/__tests__ ↔ plugin, packages/store → ui). dspot.4 on the same app finished in 208 s at 4,053 MiB, with no such warnings.

Likely cause: _select_tsconfig_programs runs existing = await digest_to_snapshot(process.input_digest) for every discovered config, and process.input_digest includes the installed resolve's node_modules. The placeholder inventory only needs to know which candidate asset paths already exist, so compute that from the source and config digest (or digest_subset over the candidate paths) instead of snapshotting the installed tree. Please add a control that a resolve with cyclic workspace links doesn't grow discovery's memory.

@smaugho

smaugho commented Nov 12, 2026

Copy link
Copy Markdown
Member Author

The post-merge cycle finding is addressed by READY Pants #3, exact head 097d64486bde9cc719a3ae71a91397cbbfbdf1d8. Discovery inventories the source/config digest instead of the installed resolve. A red-first native fixture supplies reciprocal workspace symlinks, refuses an installed-digest snapshot safely, then proves real discovery and compilation retain both links. All 54 native compiler controls pass under strict inference, as do scoped Pants lint and tailor checks.

This awaits review, then developer-25's immutable dspot.6 release. Company pantsbuild#8497's real TypeScript gate and peak memory still need qualification after that pin; the release condition and separate cost-assistant workspace-cycle follow-up are recorded there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

owner:developer-33 PR driver: developer-33

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant