[Alexandre-T]

Pull Request

Closes #42

Summary

Decode WKT and EWKT CIRCULARSTRING as dimension-specific CircularStringInterface objects, preserving the ordered control points without approximation. Add a shared parser and factory, reuse dialect-specific coordinate handling, and translate spatial model validation failures into the decoder's InvalidArgumentException with the original cause.

Require spatial-types 1.0.0-beta, which supplies concrete circular strings. Keep COMPOUNDCURVE decoding out of scope and preserve its existing NotYetImplementedException despite its implementation in the newer model.

Acceptance criteria

• All acceptance criteria are implemented.
• EMPTY, single and multiple arcs, closed circles, collinear arcs, and XY/XYZ/XYM/XYZM preserve their geometry semantics.
• EWKT SRID and dimension extensions remain supported; WKT continues to reject SRID, compact markers, and unmarked non-XY tuples.
• Nested WKT collections inherit explicitly declared ancestor dimensions, never sibling dimensions; EWKT retains existing propagation. Nested collections inherit SRIDs. Invalid syntax, control-point counts, repeated adjacent points, inconsistent dimensions and nested SRIDs are rejected.

Validation

• Relevant tests pass: 422 tests, 2,319 assertions on PHP 8.5.11.
• Repository quality checks pass: PHP CS Fixer, PHPStan level 9, PHPMD and PHPCS.
• Regression coverage preserves existing types and unsupported-type exception behavior.

Commands run

composer update longitude-one/spatial-types --no-interaction --no-progress
composer validate
composer test
composer quality
git diff --check

Composer validation succeeds with the advisory about an exact version constraint, retaining the repository's pinning policy. Quality tools emit dependency deprecations under PHP 8.5 but exit successfully.

Standards

• Applicable normative requirements were verified.

Normative references

ISO/IEC CD 13249-3:201x(E), available Committee Draft dated 2009-01-16: clause 5.1.57, circular-string grammar and Description 1(e); clause 7.3.1, Description rules 4–12. Source inspected locally. Domain invariants remain delegated to spatial-types. WKT collection inheritance now distinguishes explicit ancestor markers from sibling dimensions, following clause 5.1.57 Description 1(aa), 1(ac). EWKT inference and propagation remain unchanged.

Architecture

• No architectural boundary is affected.

The shared textual decoder handles syntax and object construction; spatial-types owns geometric invariants.

Compatibility

• Public API changes were explicitly approved.
• No breaking decoder API change is introduced.

The requested WKT restriction also corrects the pre-existing sibling propagation path for other geometry types (including POINT and EMPTY members); previously accepted implicit non-XY WKT is now rejected. No valid explicit ancestor inheritance or EWKT inference is removed.

API Gate approved in #42 (comment). The dependency pin moves from 0.0.1-alpha.4 to 1.0.0-beta to obtain CircularString; existing decoder tests pass with that dependency.

Documentation

• Documentation was updated where required.

README documents supported curves, dimensions, errors, source provenance and remaining unsupported types.

Developer handoff

Independent review and the owner's merge decision remain pending. The issue must remain In review until review, CI and merge requirements are satisfied.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.39%. Comparing base (e17796d) to head (dbacf00).

Additional details and impacted files

@@             Coverage Diff              @@
##               main      #44      +/-   ##
============================================
+ Coverage     99.14%   99.39%   +0.24%     
- Complexity      222      234      +12     
============================================
  Files            27       29       +2     
  Lines           469      492      +23     
============================================
+ Hits            465      489      +24     
+ Misses            4        3       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Alexandre-T]

Independent review — CHANGES REQUIRED

Reviewed revision: ee4f1ee52af7fda0037b374a9e8e6b2bbe21a0cf (PR head independently checked against GitHub). Issue #42, its acceptance criteria and API approval were read. No applicable AGENTS.md was found in the repository or ancestor directories. Review follows LongitudeOne spatial governance, standards, serialization and architecture skills.

BLOCKING — WKT accepts unmarked non-XY CircularString through a preceding sibling

Affected path: CircularStringWktParser::parse() lines 54–63, via GeometryCollectionWktParser::parse() lines 68–74. The collection stores the first member's dimension and passes it to subsequent members as though it were explicitly declared on the parent.

Independently reproduced with WktDecoderStrategy::decode():

GEOMETRYCOLLECTION (POINT Z EMPTY,CIRCULARSTRING (0 0 2,1 1 3,2 0 4))
GEOMETRYCOLLECTION (POINT ZM EMPTY,CIRCULARSTRING (0 0 2 3,1 1 3 4,2 0 4 5))
GEOMETRYCOLLECTION (POINT M EMPTY,GEOMETRYCOLLECTION (CIRCULARSTRING (0 0 2,1 1 3,2 0 4)))

All are accepted as XYZ, XYZM and XYM respectively, despite neither the CircularString nor an ancestor collection declaring a dimension. The dimension originates from a sibling, not an explicitly marked parent. Conversely, an unmarked CircularString as the first member is correctly rejected for three/four ordinates. Acceptance therefore depends on member order. The same path can assign a sibling's dimension to unmarked EMPTY curves.

This fails the user's explicit review requirement that WKT must reject implicit dimensions and issue #42's criterion that dimension markers follow the applicable dialect grammar. Root rejection is implemented and tested, but does not establish collection correctness. ISO/IEC CD 13249-3:201x(E), clause 5.1.57 Description 1(aa), 1(ac), distinguishes marked dimensions from the unmarked 2D case; a preceding sibling is not a dimensional declaration on the curve or its ancestors. Explicit ancestor inheritance is an existing project behavior and is distinct from this sibling propagation.

The underlying collection implementation predates this PR (both collection parser and coordinate reader are unchanged against parent e17796d). The PR newly exposes CircularString to that path; this is not claimed as a newly introduced general collection regression. Existing POINT also demonstrates the same sibling propagation.

Expected correction: distinguish explicit/inherited ancestor dimension from the dimension resolved for collection consistency, and prevent the latter from authorizing unmarked non-XY WKT members. Preserve the approved behavior of explicitly marked ancestor collections and EWKT inference. Add regressions for Z/M/ZM siblings, nested collections and EMPTY; retain positive tests for marked-parent inheritance and EWKT inference. If changing pre-existing acceptance beyond the user's requested WKT restriction requires a compatibility decision, record that decision before broadening the correction.

Verified behavior and pre-merge Definition of Done

• Root WKT rejects unmarked 3/4-ordinate CircularString and compact Z/M/ZM markers; EWKT accepts them and resolves three ordinates to XYZ, four to XYZM. M remains distinct from Z. Explicit parent GEOMETRYCOLLECTION Z/M/ZM inheritance is supported.
• Existing tests verify ordered control points, EMPTY, single/multiple arcs, closed circles, collinear arcs, XY/XYZ/XYM/XYZM, root EWKT SRID propagation, WKT SRID rejection, malformed syntax, invalid counts, repeated adjacent points, mismatched dimensions, nested SRID rejection, and wrapped model-validation exceptions.
• Independently ran composer test: 368 tests, 2095 assertions, PHP 8.5.11. PHP CS Fixer dry-run, PHPStan level 9, PHPMD for lib/tests, PHPCS, composer validate and git diff --check passed. Composer reports the existing exact-pin advisory; quality dependencies emit PHP 8.5 deprecations. No production/test source was edited.
• GitHub CI at reviewed head: PHP 8.4 and 8.5 workflows and GitGuardian successful.
• Dependency installed and required is spatial-types 1.0.0-beta (ceaee5d), with CircularString available; COMPOUNDCURVE remains explicitly unsupported. No new dependency direction or domain-model responsibility is introduced.
• README covers added behavior and dependency. API Gate approval is recorded in issue Story: Support CircularString in WKT and EWKT decoding #42. ISO circular-string grammar and model rules were inspected in the local Committee Draft: 5.1.57 and 7.3.1 Description 4–12. No unrelated architecture migration was found.
• Pre-merge functional completion and dimensional correctness are not satisfied because of the blocking finding above. Passing checks do not override that finding. No merge performed; issue must not be marked Done.

Outcome: CHANGES REQUIRED.

[Alexandre-T]

Developer correction of the blocking review finding

Addresses #44 (comment) in commit dbacf00526c4abb25a34b8986c9b5b06c7003135.

The collection parser now keeps its explicit/inherited ancestor dimension separate from the effective dimension used to enforce collection consistency. WKT only passes the former to members. EWKT continues to pass the effective dimension, preserving existing inference, measured dimensions and EMPTY propagation.

All three reproductions in the review now fail with the decoder InvalidArgumentException in WKT. The fix applies recursively and also prevents the same pre-existing sibling propagation for POINT and EMPTY members, as required by the requested WKT restriction. Explicit Z/M/ZM ancestor inheritance and individually marked siblings remain accepted. No public decoder API, dependency or domain-model change was needed.

Added 54 tests covering Z/M/ZM, both member orders, nested collections, non-empty/EMPTY curves and points, positive explicit inheritance/member markers, EWKT propagation, inferred XYZ/XYZM and SRID preservation. Before the fix, this regression suite produced 24 failures; after the fix, the complete suite passes: 422 tests, 2,319 assertions (PHP 8.5.11).

composer quality passes all checks (PHP CS Fixer, PHPStan level 9, PHPMD, PHPCS); git diff --check passes. Existing quality-tool dependency deprecations remain. README and PR description document the clarified WKT collection behavior.

Normative evidence checked locally: ISO/IEC CD 13249-3:201x(E), clause 5.1.57 Description 1(aa), 1(ac). Explicit ancestor inheritance remains the existing project contract. No unresolved Gate identified. Ready for a new independent review of this commit; no self-approval or merge.

[Alexandre-T]

Independent re-review — READY FOR MERGE

Reviewed revision: dbacf00526c4abb25a34b8986c9b5b06c7003135, independently confirmed as the GitHub PR head. This verdict supersedes my previous CHANGES REQUIRED review at #44 (comment).

The blocking sibling-dimension finding is resolved. GeometryCollectionWktParser now distinguishes the explicitly declared/inherited ancestor dimension from the effective collection dimension used for consistency. WKT passes only the former to members; EWKT preserves its existing propagation. The factory explicitly supplies the dialect.

Verified against the correction and tests:

• Unmarked WKT 3/4-ordinate curves remain rejected at the root and within unmarked collections. A preceding Z/M/ZM sibling, including EMPTY, no longer authorizes an unmarked curve or nested member. Unmarked EMPTY cannot silently acquire a non-XY sibling dimension. Both member orders are covered.
• Explicit Z/M/ZM markers on the member or an ancestor collection retain their supported behavior, including nested collections and EMPTY.
• EWKT still infers XYZ from three ordinates and XYZM from four, preserves explicit M and existing collection propagation, and preserves SRID. Compact/separate marker and WKT SRID rejection coverage remains passing.
• Independently exercised 54 additional checks outside the committed suite for LINESTRING, MULTIPOINT and MULTILINESTRING across Z/M/ZM and direct/nested members: WKT sibling propagation rejected, explicitly marked ancestor inheritance accepted, EWKT behavior retained.
• Read the 54 added test cases and reran the full suite: 422 tests, 2319 assertions passed, PHP 8.5.11. Independently reran PHP CS Fixer in dry-run mode, PHPStan level 9, PHPMD for lib/tests, PHPCS and git diff --check: all passed. I did not independently replay the reported pre-fix 24 failures.
• GitHub checks at this exact revision are successful: PHP 8.4, PHP 8.5 and GitGuardian.

Pre-merge Definition of Done: issue #42 acceptance criteria, circular-string semantics and exception handling, dialect separation, dimensional correctness, regression coverage, documentation, required quality checks and CI are satisfied. The README now documents the WKT sibling rejection and retained EWKT behavior. The correction stays inside the explicitly requested WKT restriction; no additional API/architecture/normative decision is required. Prior review of the dependency, API approval, architectural boundaries and local ISO Committee Draft sources remains applicable.

No blocking findings or unresolved gates remain. No production/test source was modified during review. No merge or project-status change was performed. READY FOR MERGE is a review outcome, not Done: owner merge decision and post-merge Definition of Done verification remain required.

Published as a comment because GitHub does not permit a formal review from the account that authored this PR; this review was conducted by the independent Reviewer agent.