Files
agiledancers/specs/S7-validation.md
Wanja Krah aae5a97200
Some checks failed
Build, test and publish / verify (push) Failing after 36s
S7 review findings
2026-10-30 23:39:14 +01:00

9.7 KiB
Raw Blame History

S7 implementation and validation evidence

Status: IN_REVIEW — implementation complete; awaiting project review. Date: 2026-10-30. Environment: Ubuntu amd64, OpenJDK 25.0.4.1, Maven 3.9.16.

Delivered changes

Enforcement/configuration: root coding standards and agent discovery instructions; project review companion (upstream skills unchanged); individually selected Checkstyle 12.3.1 checks; PMD/CPD 7.28.0 with targeted measurements and regression fixtures; JaCoCo 0.8.14 with exact line-count gates; verify integration; Docker build inputs; CI instrumentation, merge and diagnostic retention. No broad standards/coverage exclusions or unrelated rule bundles were introduced.

Migration: explicit imports and visibility, required finality and explicit types; an explicit collaborator constructor for scheduling tests while preserving CDI field injection and the public no-argument constructor; smaller snapshot-restoration, import-rebuilding, solver and validation methods; smaller HTTP test helpers; null-tolerant defensive collection copies for records, including both levels of solver assignments. Existing entity classes retain their mutation, Lombok and framework contracts. API paths, payloads, snapshot version and persistence behavior are unchanged. Enforcement and migration can be reviewed separately by path.

Scope, tools and measurements

58 handwritten production files and 9 handwritten test files are checked. Generated target output, frontend code and Java helpers outside the backend are outside that standards scope. All handwritten production packages participate in coverage, including DTOs and configuration. JaCoCo's generated-member filtering is used; no handwritten class/package exclusions are configured.

See CODING_STANDARDS.md for complete targets, strict thresholds, algorithm options, manual rules, scope and standalone commands. The full current metrics report is agiledancers-be/target/standards/report.txt. Production CPD currently reports 0% duplicated code lines at its 100-token minimum. Test duplication is not gated.

The selected PMD finality rules complement custom checks. PMD cyclomatic and cognitive visitors are scoped explicitly: lambdas contribute, nested/local type methods are independent, and no aggregate class complexity gate is introduced. Resolved collaborator identity includes qualifier values/defaults and inherited generic type substitution. Exact metric waivers are supported and regression tested; tooling/standards/waivers.tsv records the sole approved waiver: the exact eight-parameter Group constructor needed to preserve its version-2 JSON shape.

Reproducible validation

Set JAVA_HOME to JDK 25 and run from the repository root.

Command/check Result
mvn -Dformat.mode=check clean verify Passed: 38 backend tests, 19 checker tests, standards checks and the exact backend coverage gate.
mvn -pl agiledancers-be package -DskipTests Passed; Quarkus packaged app successfully produced and subsequently exercised by Playwright.
mvn -f tooling/standards/pom.xml test exec:java 19 checker tests passed; no unwaived blocking violations.
java tooling/coverage/Coverage.java agiledancers-be/target/site/jacoco/jacoco.xml Clean backend-only line gate passed: 791 covered / 818 total = 96.69926650366749%.
PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/opt/google/chrome/chrome npm run e2e in frontend Both browser tests passed against the packaged instrumented backend.
Pinned CLI dump --address 127.0.0.1 --destfile agiledancers-be/target/playwright.exec Successfully collected execution data from the agent's TCP server.
mvn -pl agiledancers-be jacoco:merge@combined jacoco:report@combined-report exec:exec@combined-coverage Passed with matching clean-build application classes: combined 791/818 lines, 96.69926650366749%.
java ci/Ci.java test 17 CI orchestration tests passed.
git diff --check Passed.

Both local and combined branch reports contain 195 covered / 257 total branches (75.87548638132296%); branches have no blocking percentage. Browser coverage is successfully collected and merged, but does not add uncovered lines in this run. Local and combined HTML/XML/CSV reports are under agiledancers-be/target/site. Execution data is in target/jacoco.exec, target/playwright.exec and target/jacoco-combined.exec within the backend module. CI copies these reports, execution data and checker results into its retained diagnostics. Local validation exercised collection and merge; a remote Gitea publication run was not performed.

The checker fixtures cover strict complexity, length, parameter, nesting, inheritance and dependency boundaries; implicit/compact record constructors; lambda/nested-type scope; comments/brace lines; distinct type fan-out; nontrivial accessors; qualifiers, repeated and inherited injection; exact waivers; parser failure diagnostics; report-only behavior; CPD occurrence/overlap accounting and test exclusion; and the unrounded 80% line gate, including a near miss.

Retained review signals

S7-review-signals.txt records the current individual REVIEW and DESIGN entries. The following assessment covers every retained review-signal group. The approved constructor waiver is documented separately below.

Signal/location Assessment and reason retained
DanceGroup and DanceSchool fan-out 16 Domain collection/entity references and explicitly used Lombok annotations are intrinsic to these mutable aggregates. Converting them to records would break their mutation/inheritance contracts.
Five REST resources, fan-out 20–25 HTTP annotations, response/request types and service/domain mapping account for the references. These are thin adapters; splitting them to reduce annotation counts would obscure endpoint ownership.
GroupView and SchoolSnapshot compact constructors, 7 parameters Their components preserve existing JSON contracts. Constructors only protect collections and are within the allowed constructor limit.
SchoolManagementService fan-out 35, public methods 28 One shared synchronization boundary owns in-memory aggregate writes and scheduling revisions. Splitting state/locks needs its own concurrency design; the class passes blocking length/dependency limits.
validateAssignments cyclomatic 8 Independent input/result guards and required-appearance validation are explicit and tested; per-part validation has already been extracted. It remains below the blocking limit.
updateShow, 33 lines and 5 parameters Retains assignments by matching daypart keys before replacing the aggregate. Keeping the transaction visible and preserving its existing signature is preferable to artificial wrappers; both blocking limits pass.
SchoolSnapshot fan-out 29; of 41 lines, restoreGroups 37, restoreShows 35 Each method performs one graph conversion phase. Explicit type/reference checks protect imported data and preserve assignment identity. Refactoring has removed the over-limit monolithic restore method.
Solver fan-out 17 OR-Tools types are confined to this adapter and its dedicated helpers. Further indirection would spread solver details without improving the domain boundary.
SchedulingTest, SchoolSnapshotTest, SnapshotCollectionsTest and SchedulingServiceTest fan-out These behavior tests intentionally exercise multiple collaborating models, services, ports, exceptions and assertions. Production architecture remains covered separately by ArchUnit.
HTTP, snapshot and scheduling test methods at 32–44 lines Arrange/action/assert and try/finally cleanup remain together. Repeated setup was extracted where methods exceeded 50 physical code lines; retained methods pass all blocking limits.
All DESIGN record-component entries Request/view/snapshot/domain records model existing named contracts; their sizes are not automatically gated. Mutable components are copied, including both Group lists under its approved constructor waiver. Framework-backed mutable entities remain classes.

Approved compatibility decisions

The maintainer approved both proposals in the S7 implementation conversation:

  1. GroupDetails groups category, type, age bounds and styles. The Java signatures of DanceGroupService.add, SchoolManagementService.createGroup and updateGroup now take 3, 2 and 3 parameters respectively. All callers were migrated. REST endpoints, JSON payloads and snapshot version remain unchanged. The record copies its styles, and service construction takes a mutable copy for the domain group; regression coverage verifies later style edits still work.
  2. The explicit eight-component service.model.Group constructor defensively copies styles and dancer IDs while preserving null values for existing import validation. Its sole waiver matches the fully resolved constructor signature, metric and exact retained value 8. The agreement and reason are recorded in tooling/standards/waivers.tsv and the result remains visible as WAIVER.

There are no parameter waivers for the three service methods, blanket suppressions or weakened thresholds. The completed specification resides in specs/in_review.

Review fixes

The review fixes enforce finality for assign-once pattern bindings, including instanceof, switch and record-deconstruction bindings, while allowing reassigned bindings and inferred record-pattern types. Trivial-accessor detection now uses AST statements and resolved field/parameter references, so comments do not change public-method counts. Accessors with calls, extra statements or compound assignments still count. Two regression tests cover these cases.

After these fixes, JDK 25 mvn -B -ntp -Dformat.mode=check verify passed with 38 backend tests, 19 checker tests and backend line coverage of 791/818 (96.70%).