Skip to content

refactor!: make the codebase smaller and gate on duplication and test-only code - #112

Merged
emaarco merged 11 commits into
mainfrom
emaarco/simplify-codebase-reduction-plan
Oct 27, 2026
Merged

emaarco merged 11 commits into
mainfrom
emaarco/simplify-codebase-reduction-plan

Conversation

@emaarco

@emaarco emaarco commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Summary

Makes the codebase measurably smaller without moving complexity around. Candidates came from a baseline of knip, jscpd, madge and change hotspots. Each was kept only if it removes net lines or concepts. Code lines in src/ (cloc) drop from 42,538 to 42,212 (−326; −397 before the last commit, which spends 71 lines on clearer services). Test code shrinks by 171 lines. Duplication in src/ falls from 21 clones (0.38 %) to 13 (0.24 %).

The baseline already showed a lean codebase: knip found nothing unused and duplication was 0.38 %. The gains are consolidations, and the size that remains is in the features.

What changes, one commit each:

  • Unused bits: the playwright devDependency (its script went in chore: remove the demo GIF script in favour of the product-shots skill #90), tools/render.sh (tools/bench/render.sh renders any file), and the it.todo gate in ops-mutate.test.ts for packages that are implemented by now.
  • CLI errors in one place: Cli.guarded is gone, because Cli.run already reports every error thrown by a command. The loadPort() / savePort() guards in five services are dropped too, since no entry calls a portless service with a file.
  • One set of geometry helpers: the metrics (harness.ts), routing.ts, geometry.ts and NotePlacer reuse geom.ts instead of their own copies (segmentHits, overlaps, containsPoint, segmentsCross, toSegments, crossings). New translate* helpers replace the hand-written move loops in FrameEditor, SpaceTool, NodePlacer, Drawing and scope.ts. PlacerState is now a Pick<NodePlacer, …>.
  • One source for shape checks: typed split ops are checked by CommandParser.parseBranches instead of a copy in BranchSplitService. detachFromParent(el, replacement?) replaces the copy in ElementTypeChangeService, and the identity map CASE_WORD is gone.
  • CLI op fields from lists: givenStrings / givenFlags replace 35 repeated ...(options['x'] ? { x: String(options['x']) } : {}) lines. CommandCatalogue becomes the functions commandDoc and commonOption.
  • Lane walk and id index shared: allLanes moves to document/utils/lanes.ts and is shared by Doc, LaneChecks and TemporaryLaneMembers, which also uses indexById.
  • Services in plain sight, ports mandatory: every port of the five document services is required, so no ! reaches into an optional one; the in-memory API passes NoDocumentFiles (adapters/out/memory), the counterpart of NoProjectSettings. Each public method reads as open, call the domain on a line of its own, write. The check of bpmn validate moves into the domain object DocumentCheck, which owns CheckOptions and CheckResult (the port re-exports them).
  • Gate: npm run gate now also runs knip --production --exclude dependencies, which fails on code in src/ that only tests reach unless the export is tagged @internal; the eight existing test hooks are tagged. It also runs jscpd with a budget of 0.25 % for src/ (the small open / target / write steps repeat in three services, since the architecture keeps helpers out of application/) (.jscpd.json, jscpd 5.4.1 as a new devDependency). knip 6 dropped its class-member check, so unused public methods are not caught automatically.

Breaking changes

  • mutationWarnings, opWarnings, renderAround, renderContext, renderMetrics, decisionLinkOf, scopeView, assertLossless, kindsSectionJson, runFormatOps, viewDoc and renderShown are no longer exported from @miragon/bpmn-cli, and layoutDocToFile no longer from @miragon/bpmn-cli/node. None of them was documented in docs/library.md, and none is used in test/, tools/, design-iq, ask-miragon, elvis or process-engine-blueprints. The use-case methods that only they reached are removed with them.
  • Flow and DI ids that bpmn-cli 0.6 cut at the 64-character limit in its own way are no longer recognised as generated, so such a flow keeps its id when its ends change. Ids written by 0.7 and later, and uncut ids of any version, are not affected.

Looked at and kept: --engine auto together with bpmn-auto-layout. On the 115 scenarios, a full redraw with each engine and the same layout metrics gives a total problem score of 410 for clean against 554 for auto. Clean is better on 39 files, auto on 22 and they tie on 54. Auto draws some diagrams cleanly where clean does not (e.g. controlflow__s23: 7 for clean, 0 for auto), so it stays as the alternative.

Checklist

  • npm run typecheck passes
  • npm run gate passes
  • src/report/guide.ts and the matching page in docs/ are updated (only if a command, option or output changed)
  • PR title is a changelog-worthy Conventional-Commit summary

Every step was checked on its own with npm run typecheck and npm run gate (2,395 tests, 246 skipped live-engine tests). The layout-regression score stayed at 370 of a 370 budget throughout. For the id change, tools/speaking-ids.mjs --baseline against the build before it gives the same new ids on all 570 probes and no renamed flow. No command, option or CLI output changed. The docs that described removed code (docs/architecture.md, docs/testing.md, CONTRIBUTING.md) are updated. Not run: the live-engine tests (no Camunda engine running) and the full bench (tools/bench/run.mjs).

Linked issue

None.

…e methods only they reached

BREAKING CHANGE: mutationWarnings, opWarnings, renderAround, renderContext, renderMetrics,
decisionLinkOf, scopeView, assertLossless, kindsSectionJson, runFormatOps, viewDoc and
renderShown are no longer exported from @miragon/bpmn-cli, and layoutDocToFile no longer
from @miragon/bpmn-cli/node. None of them was documented in docs/library.md.
BREAKING CHANGE: flow and DI ids that bpmn-cli 0.6 cut at the 64-character limit in its own
way are no longer recognised as generated, so they keep their id when their ends change.
Ids 0.7 and later wrote, and uncut ids of any version, are not affected.
@emaarco
emaarco marked this pull request as ready for review October 27, 2026 16:57
…h mandatory ports

Every port of EditDocumentService, RelayoutDiagramService, CreateDocumentService,
ValidateDocumentService and ReadDocumentService is required now, so no `!` reaches into an
optional one. The in-memory API hands them NoDocumentFiles (adapters/out/memory), the
counterpart of NoProjectSettings: nothing to read, nothing written.

The public method of each service reads as open, call the domain on a line of its own, write.
The check of `bpmn validate` moves out of ValidateDocumentService into the domain object
DocumentCheck, which owns CheckOptions and CheckResult; the port re-exports them.

The jscpd budget goes from 0.2 % to 0.25 %: the small open / target / write steps repeat in
three services, as the architecture keeps helpers out of application/.
@emaarco
emaarco merged commit 247e1e1 into main Oct 27, 2026
8 checks passed
@emaarco
emaarco deleted the emaarco/simplify-codebase-reduction-plan branch October 27, 2026 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant