Modularization and Modernization

[c-schuler]

Modularization and Modernization

Description

This PR modernizes the cqf-tooling project infrastructure across four major areas: Java version upgrade, Maven multi-module decomposition, CLI modernization, and operation pattern migration. The monolithic tooling module is decomposed into 8 focused modules with proper dependency layering, and all 23 legacy operations are migrated to the new annotation-based ExecutableOperation pattern.

• Github Issue:
• I've read the contribution guidelines
• Code compiles without errors
• Tests are created / updated
• Documentation is created / updated

By creating this PR you acknowledge that your contribution will be licensed under Apache 2.0

[bryantaustin13]

Claude review:
Verdict: right direction, real substance, not close to landable. The module decomposition is well-designed and the infra upgrades are genuine wins. But the branch has 14 failing tests that are regressions (not pre-existing), a hard-broken CLI contract with no compat path, 20 merge conflicts, and it still carries the dead HLN dependency. It also hasn't had CI run since February.

Scale and staleness

• 1,581 files, +6.57M/−34k lines vs merge-base 6d6da43; 31 commits.
• Opened 2026-02-17, last commit 2026-03-16 — ~4.5 months idle, zero review comments.
• Exactly one CI run ever (2026-02-17, commit 59001d5, failed at the Compile step on ubuntu; windows cancelled). Nothing since, so the last 7 commits are unverified by CI.
• Master is 7 commits ahead. git merge-tree reports 20 conflicts, including the nasty structural kind: modify/delete on ErsdTransformer (master's eRSD Transformer Enhancements for eRSD V3 #592 eRSD V3 work), BundleToTransactionOperation, operation/ig/Refresh; LogicDefinitionUtils (Deduplicate logicDefinition extensions on the moduleDefinitionLibrary #594) and eRSDv2_specification_bundle.json added by master inside directories this branch renamed; and a directory-rename split on tooling/src/test/java/.../utilities. Rebasing means hand-reapplying master's work into new module paths, not just resolving text.
• Branch version is still 3.11.0-SNAPSHOT; master is at 3.12.0-SNAPSHOT.

What I verified by building it

I have the (now unobtainable) mts-support-rckms-2.0.0.jar cached locally, so I could actually build the branch — note that nobody with a clean ~/.m2 can, because the branch still declares the dead hlnconsulting.jfrog.io repos and the RCKMS dep in tooling-cql. That's what PR #601 removes.

All 10 reactor modules compile on Java 17. Test census (network up, failures ignored so the reactor completes):

┌────────────────────────┬───────┬──────────┐
│ module │ tests │ failures │
├────────────────────────┼───────┼──────────┤
│ tooling-core │ 361 │ 0 │
├────────────────────────┼───────┼──────────┤
│ tooling-terminology │ 7 │ 0 │
├────────────────────────┼───────┼──────────┤
│ tooling-cql │ 52 │ 4 │
├────────────────────────┼───────┼──────────┤
│ tooling-casereporting │ 7 │ 1 │
├────────────────────────┼───────┼──────────┤
│ tooling-acceleratorkit │ 9 │ 0 │
├────────────────────────┼───────┼──────────┤
│ tooling-ig │ 283 │ 9 │
├────────────────────────┼───────┼──────────┤
│ tooling-cli │ 0 │ — │
├────────────────────────┼───────┼──────────┤
│ total │ 719 │ 14 │
└────────────────────────┴───────┴──────────┘

I baselined the same tests on master: BundleToResourcesTest 2/2 pass, ECQMCreatorIT + TESPackageGeneratorIT 87/87 pass. So these are regressions introduced by the branch, not inherited breakage.

Blockers

1. TES fixtures landed in the wrong module. TES_Groupers.xlsx, valueset-rckms-condition-codes.json, and the 44 MB reporting-specification-groupers-bundle.json are in tooling-ig/src/test/resources/casereporting/tes/, but the test that reads them lives in tooling-casereporting — whose tes/ dir contains only output/. Result: TESPackageGeneratorIT expected 618 entries, got 1. Two fixes: move the three files, and make TESPackageGenerator fail loudly on unreadable inputs instead of silently emitting a 1-entry package.

2. tooling-cql is missing an ELM writer implementation. 3 × LibraryGenerationIT fail with No ElmLibraryWriterProviders found on the classpath... add elm-jackson or elm-jaxb. tooling-cql declares elm and elm-fhir but not elm-jaxb; only tooling-ig and tooling-cli do. The CLI works by transitive luck — but anyone consuming tooling-cql as a library gets a broken library-generation path.

3. ExtractMATBundle silently dropped positional-argument support. 7 × ECQMCreatorIT fail with Missing required parameter: -ptb | -pathtobundle. Master's legacy op accepted {"-ExtractMATBundle", ""} — a bare positional path. The new @OperationParam version requires -ptb=, and the adapter silently discards tokens without =. The tests are the visible symptom; the user-facing contract change is the real issue and it's undocumented.

4. -BundleToResources swapped implementations with different behavior. Master's CLI dispatches to the legacy operation.BundleToResources (-p=, writes -.json). The branch deletes it and routes to operations.bundle.BundleToResources (-ptb=, writes -.json). Two user-visible changes — arg alias and output filename layout — that downstream IG scripts consume. Worse, the test was edited to accept a supposed regression with this comment:

▎ // Note: on case-insensitive file systems (macOS), Library "LibraryEvaluationTest" and Questionnaire "libraryevaluationtest" collide, resulting in 4 files instead of 5

That diagnosis is wrong. I inspected the actual output: 5 distinct files, because the version suffix disambiguates (LibraryEvaluationTest-1.0.000.json vs libraryevaluationtest-1.0.0.json). The test fails simply because its expectations omit the version suffix. Please fix the expectations and delete the comment — as written it enshrines a phantom data-loss bug as intended behavior.

Same category, worth auditing across the whole branch: the "consolidate duplicate operations" commits swap implementations that are not behaviorally equivalent. Each consolidated pair deserves a characterization test against master's behavior, or an explicit note that the behavior changed.

5. Land Remove GenerateCQLFromDrool and VmrToFhir operations to drop the offline HLN repository #601 first. The branch carries all 103 drool/vMR files in tooling-cql plus the dead HLN repos. Rebasing after Remove GenerateCQLFromDrool and VmrToFhir operations to drop the offline HLN repository #601 removes them, which also resolves the 4th cql failure (HtmlFileVisitorIT → NoClassDefFoundError: org/apache/log4j/Logger) for free.

Design questions better settled now than later

6. The CLI break has no migration path. java -jar tooling-cli.jar -RefreshIG -ini=... now fails outright; the new form is ig refresh -- -ini=.... Every IG _refresh script, CI pipeline, and downstream automation breaks with no deprecation window. Recommend a passthrough shim in Main (if args[0] matches a known legacy operation name, route to the old path and print a deprecation warning) plus a 4.0.0 version, given this and Quick tooling changes - R4 support, added links for Interval<T>, upda… #3/Generate model info #4.

7. picocli is only being used as a router. Leaf commands collect a raw List after -- and re-prepend -OperationName, so you get none of picocli's actual benefits — no per-arg validation, no typo detection, no generated help or shell completion for the args users care about. The @OperationParam metadata is already machine-readable; generating a CommandSpec from it would give real parsing and let the awkward -- separator go away.

8. The adapter is more permissive than what it replaces. Master's OperationFactory.processArgs throws InvalidOperationArgs on an arg without =. ExecutableOperationAdapter.parseArgs skips it silently, so a typo now yields default values and a wrong-but-successful run. Also: the adapter applies defaultValue before checking required (master checks required first), so a required param with a default no longer errors; and toLowerCase() should be toLowerCase(Locale.ROOT). Error handling in OperationCommand.call() prints e.getMessage() and discards the stack trace — rough for field debugging, and empty output on an NPE.

9. The tooling aggregator isn't drop-in backward compatible. It's pom, so an existing Maven consumer of org.opencds.cqf:tooling fails to resolve (tooling-3.x.jar doesn't exist) unless they add pom. If the goal is a no-op upgrade for consumers, publish a shaded jar at the old coordinate; otherwise the description should say "requires a POM-type dependency change" and the release notes should spell out the migration.

10. Test coverage is missing on precisely the new code. tooling-cli's only test class has every method @ignore("Skipped until operation refactor is ready") — the entire 13-class picocli layer runs zero tests. tooling-modelinfo has no test directory at all (34 classes). Meanwhile spotless is configured but bound to no goal, so formatting isn't actually enforced. (checkstyle:check is bound to validate and passes.)

11. ~174 MB of duplicated fixtures. tooling-ig/src/test/resources/casereporting/ duplicates 11 files verbatim from tooling-casereporting, and 3 acceleratorkit .xlsx fixtures exist in both tooling-ig and tooling-acceleratorkit. Worth clearing out, especially while Remove GenerateCQLFromDrool and VmrToFhir operations to drop the offline HLN repository #601 is shedding weight for the same reason.

What's genuinely good

• The module DAG is clean and acyclic — I mapped every inter-module dep: core → {terminology, modelinfo, cql} → {casereporting, acceleratorkit} → ig → cli. No cycles, sensible layering, and core depends on nothing internal. That's the hard part of a decomposition and it's right. (Minor: tooling-cli declares only tooling-ig and leans on transitive resolution.)
• 361 new tooling-core tests covering IOUtils, CanonicalUtils, and the adapter — code that previously had none, and they all pass.
• Java 17, .editorconfig, checkstyle, errorprone, jacoco, enforcer: real infrastructure improvement. Dropping Java 11 is a consumer-visible change worth a release note (CI already runs 21).
• Good hygiene throughout: removing tooling-ui, deleting dead argument processors and orphaned parameter classes, per-package READMEs, the compatators → comparators spelling fix, Paths.get() over manual separators.

Suggested path to landing

1. Merge Remove GenerateCQLFromDrool and VmrToFhir operations to drop the offline HLN repository #601. 2. Rebase; drop drool/vMR from tooling-cql; bump the version. 3. Reapply master's 7 commits into the new module paths (~7 files, the conflict list above). 4. Fix the 14 tests — move TES fixtures, add elm-jaxb to tooling-cql, correct the BundleToResources expectations and comment, update the ExtractMAT arg form. 5. Decide the CLI compat story (Library narrative #6) and version accordingly. 6. Un-ignore the CLI tests. 7. Get CI green on both OSes.

Process suggestion worth more than any single item above: split this into stacked PRs — (a) Java 17 + build/quality tooling, (b) module decomposition with no behavior change, (c) operation-pattern migration, (d) the picocli CLI rewrite. A 1,581-file draft has sat 4.5 months without a single review comment, which is itself evidence it's too big to review. Each of those four lands independently and (b