V5 breaking changes

[allanbowe]

feat!: apply planned v5 breaking changes

Summary

This PR implements all the breaking changes listed in the README "Breaking Changes" section, which were planned for the next major release (v5). All of these changes were previously marked with the @deprecated doxygen tag or documented as upcoming.

Breaking Changes

1. mcf_xxx macros — insert_cmplib= removed, wrap= defaults to YES

The insert_cmplib= option has been removed from:

• fcmp/mcf_getfmttype.sas
• fcmp/mcf_length.sas
• fcmp/mcf_string2file.sas

The CMPLIB option is now checked automatically, with the value inserted only if needed, so the parameter was redundant. Additionally, the wrap= option now defaults to YES (previously NO) — set it explicitly to NO if you rely on the old behaviour.

Callers and tests updated accordingly (mp_ds2csv.sas, mp_getmaxvarlengths.sas, mcf_getfmttype.test.sas, mcf_length.test.sas, mcf_string2file.test.sas).

Migration: remove insert_cmplib= from your %mcf_*() calls, and explicitly set wrap=NO if you do not want the proc fcmp wrapper.

2. mf_getuniquelibref.sas — maxtries= removed

The deprecated maxtries= parameter has been removed (it was ignored — the value is computed automatically from the prefix length).

Migration: remove maxtries= from your %mf_getuniquelibref() calls.

3. mp_abort.sas — type= removed

The redundant type= parameter has been removed. It was not referenced anywhere in the macro logic or by any caller in this library.

Migration: remove type= from your %mp_abort() calls.

4. mp_coretable.sas — removed

Replaced by the standalone macros in the ddl folder:

Old call	Replacement
%mp_coretable(DIFFTABLE, libds=...)	%mddl_dc_difftable(libds=...)
%mp_coretable(FILTER_DETAIL, libds=...)	%mddl_dc_filterdetail(libds=...)
%mp_coretable(FILTER_SUMMARY, libds=...)	%mddl_dc_filtersummary(libds=...)
%mp_coretable(LOCKTABLE, libds=...)	%mddl_dc_locktable(libds=...)
%mp_coretable(MAXKEYTABLE, libds=...)	%mddl_dc_maxkeytable(libds=...)

Documentation references in mp_stackdiffs.sas, mp_storediffs.sas, mp_retainedkey.sas and mp_filterstore.sas were updated, and the dependent tests (mp_filterstore.test.1/2.sas, mp_lockanytable.test.sas) now call the mddl_dc_* macros directly. mp_coretable.test.sas was removed.

5. mp_getddl.sas — renamed to mp_ds2ddl.sas

Renamed for consistency with the other ds2xxx macros. The previous mp_ds2ddl.sas wrapper was removed and the full implementation now lives in mp_ds2ddl.sas. The showlog= default is now YES (previously NO).

References updated in mp_lib2inserts.sas, mp_dsmeta.sas, and the tests (mp_getddl.test.sas → mp_ds2ddl.test.sas, mp_lib2inserts.test.sas).

Migration: replace %mp_getddl(libref, ds, ...) with %mp_ds2ddl(libref, ds, ...), and set showlog=NO explicitly if you relied on the old default.

6. mp_testservice.sas — renamed to mp_execute.sas

The macro doesn't actually test anything — it executes a SASjs web service on SAS 9 or Viya (wrapping mx_testservice.sas). The signature is unchanged.

Migration: replace %mp_testservice(...) with %mp_execute(...).

Other changes

• README "Breaking Changes" section rewritten to describe the changes as applied in v5.
• README Coding Standards and AGENTS.md updated to document the existing hasMacroNameInMend lint rule (the closing %mend must contain the macro name). The README previously stated the opposite.

Verification

• sasjs lint passes on all files.
• Generated files (all.sas, mc_*.sas) intentionally untouched — rebuilt by CI/CD.
• Tests require a real SAS server (sasjs test) — to be run via CI.

Versioning

The commit uses the feat!: convention with a BREAKING CHANGE: footer, so semantic-release will cut v5.0.0 on merge.

[allanbowe]

Hermes Agent Code Review

Verdict: Comment - No blocking issues. The PR is well-structured and the breaking changes are clearly documented. A few observations below.

Summary

This PR applies all planned v5 breaking changes: removing deprecated parameters (insert_cmplib=, maxtries=, type=), removing mp_coretable.sas (replaced by mddl_dc_* macros), renaming mp_getddl.sas to mp_ds2ddl.sas, and renaming mp_testservice.sas to mp_execute.sas. 30 files changed across macros, tests, docs, and the generated all.sas.

What's done well

• Clean rename of mp_getddl -> mp_ds2ddl: I diffed the old mp_getddl.sas body against the new mp_ds2ddl.sas body - the implementation is byte-identical apart from the macro name, file name, and showlog default. No logic was lost or silently changed in the move.
• Latent bug fixed: The old mp_ds2ddl.sas wrapper hardcoded flavour=SAS when delegating to mp_getddl, silently ignoring the caller's flavour= parameter. The merged version correctly passes &flavour through. This is a positive side effect of the merge.
• No stale references: I searched all of base/, server/, tests/, and fcmp/ - there are zero remaining references to mp_getddl, mp_testservice, mp_coretable, or insert_cmplib. All callers and docs were updated.
• mf_getuniquelibref cleanup is correct: maxtries is now a %local variable computed from the prefix length, covering the full range of possible librefs. The deprecated parameter was indeed redundant.
• mp_abort type= removal is safe: Confirmed type was never referenced anywhere in the macro body - it was a dead parameter.
• Test updates are thorough: All affected tests (mcf_*.test.sas, mp_filterstore.test.1/2.sas, mp_lockanytable.test.sas, mp_lib2inserts.test.sas) were updated to use the new macro names and signatures.
• README and AGENTS.md updates are accurate: The breaking changes section was rewritten from future-tense to past-tense, and the hasMacroNameInMend lint rule is now correctly documented.

Observations (non-blocking)

1. showlog=YES default has a ripple effect: mp_lib2inserts.sas calls %mp_ds2ddl(sashelp, schema=work, fref=tempref) without setting showlog=. With the old wrapper (which defaulted to NO), this was silent. Now it will dump DDL to the log. If that's undesired in the mp_lib2inserts context, set showlog=NO explicitly there. (See inline comment on mp_ds2ddl.sas.)

2. PGSQL flavour is undocumented: The flavour= parameter docs list only SAS and TSQL, but the macro body handles PGSQL too. Pre-existing, but v5 is a good time to document it. (See inline comment.)

3. Test coverage is thin for the merged macro: mp_ds2ddl.test.sas only tests flavour=TSQL. The SAS (default) and PGSQL code paths are untested. Pre-existing from mp_getddl.test.sas, but worth improving. Also, the test file is missing a trailing newline.

4. No test for mp_execute: The renamed mp_testservice -> mp_execute has no test (requires a live SASjs server). Understandable, just noting.

Security & correctness

• No security concerns: no user input handling, no injection vectors, no secrets.
• No logic errors introduced: all changes are parameter removals, renames, and doc updates.
• The mddl_dc_* macros in the ddl/ folder accept libds= and are direct drop-in replacements for the old mp_coretable dispatch - confirmed they exist and have matching signatures.

---

Reviewed by Hermes Agent

[allanbowe]

[4gl-reviewer]

4gl-reviewer Incremental Code Review

Verdict: Comment — The new commits address several items from our initial review. Acknowledging the fixes below and flagging a few new observations.

✅ Fixes from our initial review (acknowledged)

1. PGSQL flavour documented — @li PGSQL added to the flavour= parameter docs in mp_ds2ddl.sas. ✅ Fixed.
2. constraints_used scope leak fixed — Changed from %global (inside addConst) to %local (declared outside addConst, in mp_ds2ddl's scope). call symput inside the nested macro will correctly update the parent-scope variable. No more global symbol table pollution. ✅ Fixed.
3. SAS flavour test added — mp_ds2ddl.test.sas now tests flavour=SAS (the default) in addition to flavour=TSQL, with mp_assertscope guards on both. ✅ Fixed.
4. Missing newline at end of file — mp_ds2ddl.test.sas now ends with a newline. ✅ Fixed.
5. mp_execute test added — New tests/x-platform/mp_execute.test.sas creates a real web service via mx_createwebservice and executes it through mp_execute, verifying the dataset round-trips correctly. Good use of mp_assertscope with an ignorelist for JSON libname engine vars. ✅ Fixed.
6. mp_validatecol FORMAT regex improved — Now accepts numeric format names (4., 8.2, 12.) via the [0-9]+ alternation. I validated the regex against all 18 test cases (8 good, 10 bad) — all pass correctly. (This change was merged to main via PR #435 and is not part of this PR's diff, but the test cases in mp_validatecol.test.sas were updated in this PR.) ✅ Fixed.

⚠️ New observations (non-blocking)

1. Dead code in mp_searchdata.test.sas — The assertion was changed from iftrue=("&warntxt" = "") to iftrue=("%superq(syswarningtext)"="%superq(initwarningtext)"), which is more robust. However, the warntxt block (the %let warntxt=... and data _null_ step that conditionally clears it) is now dead code — the new assertion never references &warntxt. Consider removing the warntxt block for clarity. (See inline comment.)

2. PGSQL flavour still untested — While PGSQL is now documented in the param docs (fixing our previous comment), there is still no test for the flavour=PGSQL code path in mp_ds2ddl.test.sas. The PGSQL branch has significant unique logic (schema detection via dictionary.libnames, 1600-column Postgres limit, PostgreSQL-specific DDL syntax). A basic smoke test would be valuable. (See inline comment.)

3. CI workflow: skills update without error handling — The new CI step runs npx --yes skills update --project --yes without any error guard. If the skills npm package is unavailable or the sasjs/skills repo is temporarily inaccessible, the workflow step will fail and block the branch push for all.sas updates (since it's in the same job). (See inline comment.)

4. showlog=YES default ripple on mp_lib2inserts — This was flagged in our initial review. mp_lib2inserts.sas:8 still calls %mp_ds2ddl(sashelp, schema=work, fref=tempref) without setting showlog=NO. The verbose DDL output in the log from this internal call may be undesired. Still not addressed — noting again as non-blocking.

✅ What's done well in the new commits

• mp_validatecol regex fix is correct — The non-capturing group (?:[_a-z\$]\w{0,31}|[0-9]+) cleanly handles both named and numeric format names. Test cases are comprehensive (8 valid, 10 invalid patterns).
• mp_execute test is well-structured — Creates a real service, sends data through it, and verifies the round-trip. The mp_assertscope with ignorelist for JSON libname engine vars is a thoughtful touch.
• mp_assertscope additions to mp_ds2ddl test — Good practice to verify no macro variable scope leaks after the %global → %local fix.
• Lint passes clean — npx @sasjs/cli lint reports all files conform to @sasjs/lint code style.
• Test coverage at 47% (120/253 macros covered) — mp_execute, mp_ds2ddl, mp_validatecol, and mp_searchdata are all covered.
• .npmignore additions — Adding skills-lock.json and .env* to .npmignore is correct (lockfile shouldn't be published; .env* prevents accidental secret leakage).
• Agent skills are well-documented — README addition clearly explains what the skills are and how to install them. The skills-lock.json with computed hashes ensures reproducibility.

---

Incremental review by 4gl-reviewer[bot] — comparing 262a199 → 5ea28e3

[4gl-reviewer]

Hermes Agent Code Review — Follow-up

Verdict: Approve — all issues from the incremental review are resolved.

Fixes confirmed

• ✅ CI continue-on-error — notmain.yml skills update step now has continue-on-error: true with explanatory comment. A transient npm/github outage will no longer fail the workflow.
• ✅ PGSQL flavour test — mp_ds2ddl.test.sas now includes a PGSQL test block with mp_assertscope guards, covering the previously untested code path.
• ✅ Dead code removed — mp_searchdata.test.sas warntxt block (7 lines) removed. The assertion now cleanly compares against initwarningtext.

No remaining issues. LGTM.

---

Reviewed by Hermes Agent (GitHub App)

[4gl-reviewer]

Test Coverage Report

npx @sasjs/cli c run against PR branch (commit 5aab801):

Overall: 120/253 macros covered (47%)

PR-changed macros

Macro	Covered?
mp_ds2ddl	✅ Yes — tests TSQL, SAS, and PGSQL flavours with mp_assertscope
mp_execute	✅ Yes — new test in tests/x-platform/ with real web service round-trip
mp_validatecol	✅ Yes — FORMAT rule test updated with numeric format names
mp_searchdata	✅ Yes — warning assertion test with initwarningtext capture
mcf_getfmttype	✅ Yes
mcf_length	✅ Yes
mcf_string2file	✅ Yes
mf_getuniquelibref	✅ Yes
mp_ds2csv	✅ Yes
mp_filterstore	✅ Yes
mp_retainedkey	✅ Yes
mp_stackdiffs	✅ Yes
mp_storediffs	✅ Yes
mp_dsmeta	✅ Yes
mp_getmaxvarlengths	✅ Yes
mp_lib2inserts	✅ Yes
mp_abort	❌ No test (pre-existing — mp_abort.test.nofix.sas exists but is excluded from coverage)

16 of 17 PR-changed macros have test coverage. The only gap (mp_abort) is pre-existing — it requires specific runtime contexts (STP, Compute, batch) that are hard to unit-test, and the .nofix.sas test file is intentionally excluded from the coverage scan.

The new tests added in this PR (PGSQL flavour, SAS flavour, mp_execute round-trip) close the gaps flagged in the earlier review.

---

Generated by Hermes Agent (GitHub App)

[github-actions]

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀