Adopt Senzing java-coding-standards

[barrycaceres]

Summary

Adopts the shared java-coding-standards
as done in senzing-commons-java, pinned to release 0.4.1 via a
.java-coding-standards git submodule. Generated by running the standards'
adoption playbook (/init-java).

Wiring (commit ae0c5c3)

• Submodule .java-coding-standards pinned to tag 0.4.1.
• pom.xml — checkstyle profile now points at the shared
  senzing-checkstyle.xml (dropped the local <suppressionsLocation>); added
  the surefire <argLine>@{argLine} -XX:+EnableDynamicAgentLoading -Xshare:off</argLine>
  and an empty top-level <argLine> property.
• Removed the superseded blanket checkstyle-suppressions.xml.
• FAQ MCP server — .mcp.json registers sz-sdk-java-auto-faq; four starter
  project FAQs added under .claude/faqs/ (architecture, build commands,
  config-refresh modes, native-lib test setup).
• VSCode — .vscode/settings.json (format-on-save via emeraldwalk),
  tasks.json, extensions.json; .gitignore gains ! exceptions so they are
  tracked.
• Claude Code — /init-java slash command; hooks in .claude/settings.json
  (PostToolUse auto-format, Stop checkstyle, SessionStart freshness nudge).
• CLAUDE.md — added Java Coding Standards / FAQ MCP / Testing sections.

Reformat (commit d9391bf)

Mechanical formatting of all 29 files under src/main/java + src/test/java
via the shared formatter. No behavioral changes. A few lines the AST formatter
cannot auto-wrap were adjusted by hand to satisfy the now-enforced 80-column
limit (string-literal splits, two for conditions, one // CSOFF/CSON on an
unwrappable generic declaration).

Verification

• mvn -Pcheckstyle validate → BUILD SUCCESS
• mvn test-compile → BUILD SUCCESS
• Formatter idempotent (0 modifications on a second run)
• mvn test (native libs) not run in this environment — please confirm on a CI/dev box.

Reviewer / merge notes

• Restart Claude Code after merge so the sz-sdk-java-auto-faq MCP server indexes.
• Confirm the maven CI workflows check out with submodules: recursive.
• The format-on-demand keybinding is User-level and must be added by hand.

[github-actions]

Code Coverage

Overall Project	93.98% -1.27%	🟢
Files changed	92.57%	🟢

File	Coverage
SzAutoCoreEnvironment.java	95.56% -0.81%	🟢
Reinitializer.java	84.58% -3.96%	🔴

[github-actions]

🤖 Claude Code Review

Combined Code Review

Review Part 1 of 2

Heads up: .java-coding-standards has a newer release available (1.0.1); current pin is untagged. Run /init-java to refresh.

---

Code Review — Part 1 of 2

This PR adopts the Senzing Java coding standards submodule, reformats all Java source to comply, wires up tooling (VSCode, hooks, MCP FAQ server), and updates the checkstyle configuration. Review below.

---

Code Quality

✅ Style guide — Java reformatting broadly follows the Senzing standards: Allman braces on method/class bodies, same-line braces for control flow, 80-char line wrapping, operators at start of continuation lines.

❌ CSOFF/CSON misuse — SzAutoCoreEnvironment.java around the AbstractBuilder declaration:

// CSOFF
public abstract static class AbstractBuilder<E extends SzAutoCoreEnvironment,
    B extends AbstractBuilder<E, B>>
    extends SzCoreEnvironment.AbstractBuilder<E, B> implements Initializer
{
    // CSON

Per the CLAUDE.md standards: CSOFF/CSON is "only for deliberately aligned multi-line output (column-formatted diagnostics, ASCII art, SQL DDL) — never a general escape hatch." Suppressing line-length for a generic class signature is exactly the general escape hatch scenario the policy prohibits. The class signature should instead be reformatted to fit (e.g. split across lines in a compliant way without suppression).

❌ Awkward operator-split conditions — Two places produce hard-to-read continuations:

Reinitializer.java (approximately line 103 in the reformatted file):

if (duration
    == null || duration.isZero() || this.env.isDestroyed()) {

== null is stranded at the start of the continuation rather than duration on the first line and || duration.isZero()... on the next. Per the standards the operators || should lead the continuation, not ==.

SzAutoCoreEnvironment.java inside execute():

if (this.refreshMode
    == RefreshMode.DISABLED || Boolean.TRUE.equals(
        ENSURING_CONFIG.get()) || (!Boolean.TRUE.equals(
        CONFIG_RETRY_FLAG.get()))) {

The || breaks are embedded inside method calls rather than starting each logical branch. This is functionally correct but very hard to follow. Consider extracting each condition to a named local boolean before the if.

❌ Arithmetic continuation — Reinitializer.java:

long delay = duration.getSeconds()
    * ONE_THOUSAND + (duration.getNano() / ONE_MILLION);

* correctly starts the continuation, but then + (duration.getNano() / ONE_MILLION) is appended on the same continuation line. Both operators should each start their own continuation line per the "+4 per wrap level" rule.

✅ No commented-out code — cleaned up throughout.

✅ Meaningful names — no renames, existing names preserved.

✅ No defects introduced — the functional logic is unchanged; this is a pure reformat + tooling addition.

---

Testing

✅ No test logic changed — all test file changes are formatting-only.

✅ pom.xml argLine change — adding -XX:+EnableDynamicAgentLoading -Xshare:off to surefire suppresses expected JVM warnings on Java 17+. The empty <argLine/> property ensures the late-bound @{argLine} reference resolves even without -Pjacoco. Both are correct and necessary.

⚠️ No new tests — no coverage gaps introduced, but also none closed. Acceptable for a tooling/reformat PR.

---

Documentation

✅ CLAUDE.md — additions are well-structured and match what's documented in the system-level context.

✅ FAQ files — all four new files (sz-sdk-java-auto-overview.md, sz-sdk-java-auto-build-commands.md, configuration-refresh-modes.md, native-library-and-environment-setup.md) are appropriately scoped and useful.

❌ Javadoc tag in Markdown — .claude/faqs/conventions/configuration-refresh-modes.md, first paragraph:

Automatic configuration refresh keeps the
{@code activeConfigId} in sync with the current {@code defaultConfigId}.

{@code ...} is a Javadoc tag; it will render literally in Markdown. Use backtick code spans instead: `activeConfigId`.

❌ No CHANGELOG.md update — this PR deletes checkstyle-suppressions.xml, adds .java-coding-standards as a submodule, changes the checkstyle config path in pom.xml, and adds surefire JVM flags. These are user-visible build changes that should be recorded in CHANGELOG.md.

---

Security

✅ No hardcoded credentials.

✅ No .lic files.

✅ SessionStart hook — the shell snippet validates $latest and $current tag names against [A-Za-z0-9._-] before use:

case "$latest" in *[!A-Za-z0-9._-]*) exit 0 ;; esac;

Good. The ls-remote output can contain attacker-influenced data if the remote is compromised, so this guard is important.

✅ PostToolUse hook — file_path is passed as a Python argument (not interpolated into the shell command) so shell injection is not a risk. The [[ "$p" == *.java ]] guard prevents running the formatter on non-Java files.

✅ Stop hook — runs read-only Maven validation. No risk.

---

Minor / Nits

⚠️ VSCode file indentation inconsistency — .vscode/settings.json uses 4-space indentation while .vscode/extensions.json and .vscode/tasks.json use 2-space. Not a functional issue but inconsistent.

⚠️ Method-chain break in test files — BasicRetryTest.java and ConfigRetryTest.java:

features
    .values()
    .forEach((jsonVal) -> {

Breaking a two-method chain across three lines is technically standards-compliant (break before .) but adds visual noise for a trivial chain. Worth considering whether the formatter should preserve single-line chains at this length.

---

Summary

Area	Status
Java style compliance (overall)	✅
CSOFF misuse on AbstractBuilder	❌ Fix required
Awkward condition splits (3 sites)	❌ Should fix
Javadoc tag in Markdown FAQ	❌ Fix required
CHANGELOG.md not updated	❌ Should add
pom.xml improvements	✅
Hook security	✅
VSCode indentation inconsistency	⚠️ Minor

The PR is structurally sound — the submodule wiring, hook logic, FAQ server, and pom.xml changes are all correct. The blocking items are the CSOFF misuse, the condition-split formatting at a few sites, the Javadoc tag in the FAQ, and the missing CHANGELOG entry.

---

Review Part 2 of 2

Heads up: .java-coding-standards has a newer release available (1.0.1); current pin is untagged. Run /init-java to refresh.

Code Review — Part 2 of 2

This diff is a reformatting pass to adopt Senzing Java coding standards. Most changes are correct (Allman braces on method/class/constructor definitions, throws on its own line, whitespace cleanup), but there are several issues to flag.

---

Code Quality

❌ Builder-chain reformatting violates the . continuation rule

The standards doc (CLAUDE.md) states: "Operators on continuation lines: break before . (the operator starts the continuation line)."

The new builder chain style breaks that rule in every test file. Example from ConfigTest.java (~line 56):

// New (non-conforming) — breaks mid-parameter-list, '.' not at line start
this.env = SzAutoCoreEnvironment.newAutoBuilder().instanceName(
    instanceName).settings(settings).verboseLogging(false).concurrency(
    this.getConcurrency()).configRefreshPeriod(
    this.getConfigRefreshPeriod()).build();

Conforming form:

this.env = SzAutoCoreEnvironment.newAutoBuilder()
    .instanceName(instanceName)
    .settings(settings)
    .verboseLogging(false)
    .concurrency(this.getConcurrency())
    .configRefreshPeriod(this.getConfigRefreshPeriod())
    .build();

This pattern repeats in: ConfigTest.java, DiagnosticTest.java, EngineBasicsTest.java, EngineGraphTest.java, EngineHowTest.java, EngineReadTest.java, EngineWhyTest.java, EngineWriteTest.java, ProductTest.java, SzAutoCoreEnvironmentTest.java.

---

❌ Inline comments detached from their target lines — EnsureConfigRecursionTest.java

Before (~line 88):

this.bumpConfig(); // ensure config is out of sync

After:

this.bumpConfig();
// ensure config is out of sync
return super.ensureConfigCurrent();

The comment now appears between the bumpConfig() call and return, making it read as though it describes the return statement. Same issue in reinitialize() with // keep the config out of sync. These should stay as inline trailing comments.

---

✅ Allman brace placement — All method/class/constructor definitions now correctly put { on its own line.

✅ throws clauses — Correctly moved to their own line.

✅ Import group blank lines — Removed blank lines between import groups. The formatter produces this; verify with mvn -Pcheckstyle validate that checkstyle accepts it.

✅ DiagnosticTest.java annotation alignment — The pre-existing leading-space indentation bug on class annotations is correctly fixed.

✅ No commented-out code introduced.

✅ No DRY violations introduced.

---

Testing

✅ No functional changes; no new test logic was added. Existing tests are unchanged in behavior.

---

Documentation

❌ CHANGELOG.md — Not visible in this diff. If not updated, confirm this formatting-only pass is intentionally changelog-exempt.

✅ No API changes; no README/API doc updates needed.

---

Security

✅ No credentials, no input validation changes, no sensitive data in logs. Pure formatting.

---

Summary

Severity	Issue
Must Fix	Builder chains break before ), not before .; violates the coding standard in 10 files
Should Fix	Detached inline comments in EnsureConfigRecursionTest.java (lines ~88, ~96)
Verify	Run mvn -Pcheckstyle validate to confirm checkstyle accepts import-group blank line removal
Low	CHANGELOG.md update (if required for formatting PRs)

---

---

Automated code review analyzing defects and coding standards