Repository navigation
refactor(#1992): move the transpiler under src/cnx/ - #2007
Conversation
src/ now holds cnx/ (PARSE/, TRANSPILE/, WRITE/, types/, utils/, instrumentation/) beside its consumers cli/ and lib/. The move is scripts/move-modules MOVES entries applied with ts-morph; every path reference outside src/ follows. - README §1 draws the new tree; layout:check treats cnx/ as the container that holds the layers, not as a layer. - depcruise: every path rule names src/cnx/, and the new transpiler-imports-no-host rule forbids cnx/ importing cli/ or lib/. Co-Authored-By: Joshua Austill <jlaustill@gmail.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
Co-Authored-By: Joshua Austill <jlaustill@gmail.com>
- Tests under cnx/ climb one more level to the repo root, and the
join("src", ...) segments and regexes the text rewrite missed now
name cnx/.
- PlanRenderState.placeOf names the layer, not cnx/.
- Regenerated the ANTLR output with npm run antlr:all: the mover had
stripped the .js from its imports. It is byte-identical to main's.
Co-Authored-By: Joshua Austill <jlaustill@gmail.com>
…ale docs/coverage Box 4 covers tests (owner ruling 2026-10-11). 71 tests and helpers that reach the host (Transpiler, TargetCatalogFile, TargetResolver, MockFileSystem, parseCHeader, JsonCodec) compose passes, so they move to src/cli/__tests__/cnx/. Three pure helpers move into cnx/. transpiler-imports-no-host no longer skips tests. Box 5 leaves out history (owner ruling); the stale generated docs/coverage/ report is deleted (CI publishes coverage to Pages). Co-Authored-By: Joshua Austill <jlaustill@gmail.com>
jlaustill
left a comment
There was a problem hiding this comment.
Review Summary
PR: #2007 (refactor(#1992): move the transpiler under src/cnx/)
Reviewed by: Claude (automated review)
Overview
The move itself checks out. In a clean clone at 605f7a50, npx tsc --noEmit, typecheck:scripts, depcruise (0 errors) and layout:check all pass. CI unit counts went from 428 files / 8503 tests (base 1a3f0b1a) to 428 / 8507 (this head), so no test was lost. No .expected.* file changed. A grep for src/(PARSE|TRANSPILE|WRITE|types|utils|instrumentation) outside the history exclusions finds nothing. At 2026-10-11T20:53Z the head had 31 check runs passed, 2 skipped (Deploy Coverage), and 0 open Sonar issues. The findings are about the new guard's strength and about prose the rewrite made false.
Findings
| Severity | Count |
|---|---|
| Critical | 0 |
| Important | 1 |
| Moderate | 3 |
| Nit | 2 |
Key Issues
transpiler-imports-no-hostmatches direct edges only. In a mutation test, acnx/test that reachescli/TargetResolverthrough ascripts/module passed at 0 errors.reachable: truecatches it (2 errors) and leaves the clean tree at 0.layer-rules.test.tscannot hold the keyword throughLAYER_ROOTSwithout sweeping incollectors-build-names-from-scopes(measured), so the rule needs a named assertion.- History narration was rewritten to the new paths. About 18 lines now say things like "#1444 moved the port into src/cnx/PARSE/1-Discover/". The inline comment lists each one. Writing them relative to the layer keeps them true and keeps box 5's grep clean.
- Test-placement docs. CLAUDE.md:679 says both analyzer helpers live in
cnx/, and one moved tocli/. CLAUDE.md:737 andLayout.ts:14say tests sit beside their module, which no longer holds for 71 files, and nothing records where they went.
What Looks Good
- Every move is in
MOVES.tswith a reason (80 entries), so the move can be rerun on a freshmainand reviewed in the diff. - The mover defect that Verify Clean caught was filed as #2009 instead of being worked around silently, and the grammar was regenerated rather than edited.
layout.test.tsadds cases for each new failure mode: a shared root put back besidecnx/, a layer outsidecnx/, and a bare file or__tests__/directly incnx/. A README tree mutation (drawing a pass's subdirectory) also fails loudly (20 failures), so the newlayers()definition does not hide a misdrawn tree.
🤖 Automated review by Claude Code
…ive to the layer Review of #2007: - transpiler-imports-no-host is now reachable: true, held by a named assertion in layer-rules.test.ts (LAYER_ROOTS would sweep in collectors-build-names-from-scopes). - Sentences narrating earlier moves named src/cnx/ paths that did not exist then; they now name the path relative to the layer. - CLAUDE.md, AGENTS.md and Layout.ts record where a test that reaches the host lives; the analyzer-helper line names each helper's place. - README four-kinds sentence and Layout.layers() docblock corrected. Co-Authored-By: Joshua Austill <jlaustill@gmail.com>
|



Summary
src/now holdscnx/, the transpiler (PARSE/,TRANSPILE/,WRITE/,types/,utils/,instrumentation/). Its consumerscli/andlib/sit beside it. This is the owner's direction on #1313 (2026-10-11). Generated output is unchanged: no fixture.expected.*file changed, and Integration Tests and Verify Clean both pass.Changes
scripts/move-modules/MOVES.ts- adds six root entries (src/<root>→src/cnx/<root>), applied withnpm run move:modules -- --apply. That moved 1034.tsfiles and rewrote their importers. I moved the 2 oracle.hfixtures withgit mv. 4vi.mockspecifiers incli/andlib/tests were rewritten by hand.src/cnx/PARSE/2-Parse/**/grammar/, withnpm run antlr:all. The mover had stripped.jsfrom its imports. The regenerated files are byte-identical tomain's. The mover defect is filed as move:modules rewrites imports inside generated ANTLR output #2009.docs/architecture/README.md§1 - the tree now drawssrc/cnx/with the layers and shared roots, andcli/andlib/beside it. The "four kinds" paragraph says the hosts sit besidecnx/and that nothing incnx/imports them.scripts/layout/Layout.ts- a layer is now a drawn directory with no drawn subdirectory of its own. That makescnx/a container rather than a layer, and only layers may hold__tests__/..dependency-cruiser.cjs- every path rule now names^src/cnx/. The newtranspiler-imports-no-hostrule forbids^src/cnx/→^src/(cli|lib)/. Tests are not exempt (owner ruling on box 4). The rule isreachable: true, and a named assertion inlayer-rules.test.tskeeps it that way.src/cnx/that reach the host (Transpiler,TargetCatalogFile,TargetResolver,MockFileSystem,parseCHeader,JsonCodec) tosrc/cli/__tests__/cnx/, mirroring their old paths. Composing passes is the host's job (layout: src/ becomes the pass table — destination map first, cleanup PRs, then the move #1443).codeGenSymbolsHelpers,enterScopeandinstallMockSymbolsname only transpiler modules, so they move intocnx/instead. Each move is recorded inMOVES.ts.docs/coverage/, a stale generated coverage report. CI publishes coverage to Pages, so nothing reads it. Its.gitignoreand.cspell.jsonentries are gone too.scripts/plan-render-state/PlanRenderState.ts-placeOfnames the layer, notcnx/.cnx/,join("src", "cnx", …)segments, and theartifact-lifetimeregexes.package.json(antlr and clean paths),knip.json,vitest.config.ts,.oxlintrc.json,sonar-project.properties,.github/workflows/pr-checks.yml, theprettier-plugin/imports, 2tests/regressionscripts, thescripts/gates, docs,AGENTS.md,CLAUDE.md, and.claude/skills/issue-check.scripts/__tests__/layout.test.ts- adds cases for a shared root put back besidecnx/, a layer outsidecnx/, and a bare file or__tests__/incnx/.Pre-Submit Checklist
Code Quality
Documentation
learn-cnext-in-y-minutes.mdupdated (n/a, no syntax change)Git Hygiene
Testing
Mutation checks. I restored the tree after each one, and the gate went green again:
mv src/cnx/types src/types:layout:checkexits 1 with 2 failures (missingcnx/types/, unexpectedtypes/).mv src/cnx/WRITE src/WRITE:layout:checkexits 1 with 3 failures.import "../3-Render/MisraSuppressionUtils"to2-Plan/EmissionPlan.ts: depcruise reportserror 2-2-plan-reads-no-later-pass.import "../../../cli/TargetResolver"to1-Discover/NodeFileSystem.ts: depcruise reportserror transpiler-imports-no-host. The clean tree has 0 errors.scripts/__tests__/layout.test.ts: 26/26 pass.SonarCloud: 0 open issues on the PR, and the gate is OK.
Still open on #1992:
src/cnx/, tests included, importssrc/cli/orsrc/lib/. depcruise reports 0 errors. Adding acli/TargetResolverimport to acnx/test makestranspiler-imports-no-hostfail.MOVES.ts, releasedCHANGELOG.mdentries,docs/plans/,docs/archive/.Related Issues
Closes #1992. Part of #1313. Found here: #2009.
Link to Devin session: https://app.devin.ai/sessions/f8dad7b6b64443cd859a9c5715f51417
Open in Devin Desktop: https://app.devin.ai/desktop/session/f8dad7b6b64443cd859a9c5715f51417?variant=devin
Requested by: @jlaustill