fix: reject 4-item CircularString; rewrite (A,B,A) to 5-token circle

[grootstebozewolf]

HOLD merge. Off #7 (feature/sfa-curve-rgr). Do not merge to #7. Do not push to locationtech. #120 retip: four-item CIRCULARSTRING (A, B, C, A) is rejected (EX-CS-4 / ADR min ring is out). This PR does not rewrite history on #7 and does not silently revert #120 there.

Architect SIGN (PO 24 Aug 2026)

ISO/IEC 13249-3 wants an odd control count ≥ 3. No DOI. PostGIS stores odd control counts greater than 1. CIRCULARSTRING(A, B, C, A) 4-pt even is not in the PostGIS model. The JTS-only annulus exception is reverted.

1. Reject 4-item CS. CIRCULARSTRING (A, B, C, A) is not a valid stored list. #120 made 4-item valid; this PR retips that.

2. Accept CIRCULARSTRING (A, B, A) as a JTS on-ramp only. A and B must be two distinct points. Refuse A = B. This is not the ISO/IEC 13249-3 full-circle form.

3. On add/read, rewrite (A, B, A) to CIRCULARSTRING (A, C, B, D, A).

   • Diameter is A–B. O is the midpoint. R is |B−A|/2.
   • θ is the angle of vector A→B.
   • C is the point on the circle at θ − π/2.
   • D is the point on the circle at θ + π/2.
   • Result is two arcs (A, C, B) and (B, D, A). Type stays CIRCULARSTRING. Never flatten to LINESTRING.

4. MMF-IO identity is the 5-token circle, not the 3-token input. Stored list is 5 tokens, first=last, odd ≥ 3. After add/read, getGeometryType / WKT / WKB are the 5-token CIRCULARSTRING.

5. Canvas: add/load of (A, B, A) paints that circle, not a single-arc (A, B, A) and not a 4-token ring. New-draw close stays five controls. Do not add a 3-click (A, B, A) draw door. [visual-qa] 3-click CurvePolygon circle is CIRCULARSTRING(A,B,C,A) but draws a closing line #86 / fix: 3-click CurvePolygon is CIRCULARSTRING(A,B,C,A) full disc, not chord #87 stays another door. Do not invent WKT.

Follow-up (this revision)

Leftover overlay/paint consumers that still parsed 4-pt even WKT now expect ParseException. The #114 PO witness WKT is kept as-is (do not invent a fifth control). Those tests are reject-locks, not a paint remake. modules/curve/README.md no longer lists closed-4 as valid.

HOLD

Style B. #118. Do not remake #60 #82 #92 #114. Do not merge #115–#117/#119/#121 again. No RC5. Not RC4.

Do not steal claimId V-CS (Proofs locationtech#532 still certifies the old exception). NTS #19 still pins Port of JTS 2b56b1a4 (the #120 exception) and must not merge as-is.

Tests (no GUI)

Refuse A = B. Reject 4-item CS. (A, B, A) add/read rewrites to (A, C, B, D, A) and stays CIRCULARSTRING.
CurvePolygon 3-click close and CircularString new-draw close stay as they were.

  

[grootstebozewolf]

Review — reject 4-item CS; rewrite (A,B,A) to 5-token circle

Verdict: the rewrite math and the odd-≥3 stored-list rule are right. Do not merge. The #86/#87 draw door can still mint a 4-item ring that this PR will not parse back. Proofs V-CS / NTS #19 still certify the #120 exception.

CI: build-and-test and naming-guard green on HEAD 81c2e99. mergeable_state: clean. PR instruction is HOLD merge. Off #7. Do not push to locationtech.

What the letter does

Architect SIGN, 24 Aug 2026:

1. Reject stored CIRCULARSTRING (A, B, C, A).
2. Accept (A, B, A) only as a JTS on-ramp (A ≠ B).
3. On add/read, rewrite to (A, C, B, D, A): diameter A–B, centre midpoint, C at θ−π/2, D at θ+π/2. Type stays CIRCULARSTRING.
4. MMF-IO identity is the 5-token list.
5. Do not add a 3-click (A, B, A) draw door. #86 / #87 stays another door.

CircularString.isValidControlCount is now empty ∨ (odd ∧ ≥ 3). The #120 threePointCircleCloseMid exception is gone from that predicate.

Expansion on the unit test pair is exact: A=(0,0), B=(2,0) → C=(1,−1), D=(1,1). Two semicircles. Factory test with vertical diameter matches. WKT/WKB of the rewritten object stay 5-token CIRCULARSTRING. A = B throws. Four-item WKT throws ParseException.

That is the on-ramp. Tests in CircularStringDiameterOnRampTest lock it. README no longer lists closed-4 as valid.

Do not merge (PR is right about that)

#120 already landed on #7 (1e446de). This branch does not rewrite that history; it retips on a side branch. Merging now would:

• silently revert the V-CS / #120 pin that NTS #19 still cites as Port of JTS 2b56b1a4
• leave Proofs locationtech#532 certifying an exception this tree no longer implements
• land a stored-list change without an NTS remint

HOLD is the correct state. Style B. Do not remake #60 / #82 / #92 / #114.

Draw / parse split

GeometryCombiner.isValidCircularControl is not retipped:

if (pts.length % 2 == 1) return true;
return pts.length == 4 && pts[0].equals2D(pts[3]);

addCircularString / addCurvePolygon still accept a closed 4-item list, then call curveFactory().createCircularString. The factory only rewrites size == 3. Result: the model holds a 4-control CircularString with isValid() == false.

WKT load of that same list throws. So the #86 / #87 3-click close door (explicitly left alone) can still write a ring this reader will not take back. That is a broken identity, not “another door.”

Same split on construction:

Path	4-item (A,B,C,A)	3-item (A,B,A)
CurveWKTReader	ParseException	rewrite to 5
CurveGeometryFactory.createCircularString	object, isValid()==false	rewrite to 5
new CircularString(...)	object, isValid()==false	object, 3 pts, isValid()==true (odd)
CurveWKBReader (via factory)	object, invalid	rewrite to 5
GeometryCombiner.add*	adds 4-item object	factory rewrite to 5

Constructor-allows-invalid is normal JTS. Combiner still preferring closed-4 as a valid add is not. Either retip isValidCircularControl and the 3-click close to emit five controls, or the “reject 4-item” rule is only a WKT rule.

getLength / envelope / toLinear still call threePointCircleCloseMid. 4-item objects that slip through the constructor still behave as the old full circumcircle. The geometry methods were not retipped; only the count predicate and the reader were.

Tests

Useful: on-ramp rewrite, factory, WKT/WKB identity, A = B, four-item WKT reject; canvas load + GeometryEditModel.addComponent of (A,B,A) → 5 tokens; leftover overlay/paint consumers now expect ParseException on the #114 witness WKT.

Weak / gone:

• WKTCircularStringTest.testRejectsClosedFourPointCircle catches Throwable and does not check the message
• CurveShapeWriterThreePointHoleTest and LineStringStyleCurveHoleTest deleted the annulus paint locks and became reject-only. There is now no 5-token shell + 5-token hole paint test. testShapeWriterPaintsTwoArcs… only asserts cubic >= 2
• no test that Combiner / 3-click close refuses 4-item, because they do not

Non-blocking

1. Rewritten circle for eastward A→B is clockwise (A, C_south, B, D_north, A). Fine as a lineal CS. A CurvePolygon shell that uses this on-ramp is a CW ring.
2. C and D are atan2/cos/sin. Unit-diameter cases are exact; arbitrary diameters will WKT-roundtrip as ugly floats.
3. Z/M: C and D are a.copy() then setX/setY, so they inherit A’s Z. Stay 2D.
4. Double-apply is safe: WKT reader rewrites then constructs; factory rewrites only size == 3.

Out of scope (agreed)

• Merging to #7 / locationtech
• Remaking #86 / #87 draw
• Inventing a 5th control on the #114 witness
• Stealing Proofs claimId V-CS
• NTS #19 remint
• RC5