V-CP and V-CS: tests with hunter for counterexamples + impl hardening#1199
Closed
grootstebozewolf wants to merge 33 commits into
Closed
V-CP and V-CS: tests with hunter for counterexamples + impl hardening#1199grootstebozewolf wants to merge 33 commits into
grootstebozewolf wants to merge 33 commits into
Conversation
Adds .github/workflows/auto-rebase-upstream.yml which runs daily (and
on workflow_dispatch). It fetches upstream/master, rebases the fork's
master onto it, and:
* force-pushes with --force-with-lease on a clean rebase, OR
* aborts the rebase and opens / updates a labelled GitHub issue
listing the conflicting paths + a copy-paste manual recipe.
Env vars at the top of the workflow (UPSTREAM_REPO, UPSTREAM_BRANCH,
TARGET_BRANCH, CONFLICT_LABEL) are the only knobs needed to repoint
the job at a different upstream or fork branch.
Assisted-by: Claude (Opus-4.7)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two-stage chain:
Stage 1 rebases master onto upstream/master (locationtech/jts).
Stage 2 (matrix, needs: stage 1) rebases every saga branch onto the
freshly updated fork master, in parallel, with fail-fast disabled
so one conflict doesn't block the rest.
Each branch gets its own labelled conflict issue, auto-updated on
every subsequent failed run; on a clean rebase the workflow does a
--force-with-lease push back to that branch.
RULE-OUT spike branches (F-CP-spike-optionB, F-CP-spike-optionC) are
deliberately not in the matrix -- they're frozen experiments, force-
pushing them would lose the audit trail. Add them to the matrix in
this file if you want them tracked too.
Assisted-by: Claude (Opus-4.7)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduce minimal, surgical seams in jts-core so an opt-in extension module (jts-curved) can plug in support for CircularString, CompoundCurve, CurvePolygon, MultiCurve, MultiSurface, Triangle, PolyhedralSurface, and Tin without any new geometry classes or algorithm changes in core. Modelled on NetTopologySuite PR locationtech#526, which applies the same pattern in the .NET sister project. WKTReader - Add protected `readOtherGeometryText(tokenizer, type, ordinateFlags)` hook called when a type keyword is unrecognised. Default impl preserves the prior `Unknown geometry type` ParseException. - Promote tokenizer helpers from `private` to `protected`: getCoordinate, getCoordinateSequence, createCoordinateSequenceEmpty, getNextEmptyOrOpener, lookAheadWord, getNextCloserOrComma, getNextWord, parseErrorWithLine, readGeometryTaggedText (3-arg), readLineStringText, readLinearRingText, readPolygonText, readMultiPolygonText. - Promote `geometryFactory` and `csFactory` fields to protected. WKTWriter - Add protected `appendOtherGeometryTaggedText(...)` hook called early in the dispatch ladder so subclasses can intercept geometries that would otherwise be routed to a parent class's handler by `instanceof`. Default returns false, preserving prior behaviour. - Replace four hard-coded `WKTConstants.*` keyword writes with `geometry.getGeometryType().toUpperCase(Locale.ROOT)`, so subclasses with structurally compatible bodies emit their own keyword without needing dedicated dispatch branches. No-op for existing types. - Promote helpers to protected: indent, appendOrdinateText, appendSequenceText, appendPolygonText, appendMultiLineStringText, appendMultiPolygonText. WKTConstants - Add CIRCULARSTRING, COMPOUNDCURVE, CURVEPOLYGON, MULTICURVE, MULTISURFACE, POLYHEDRALSURFACE, TIN, TRIANGLE constants. Core readers and writers do not handle these directly; they are exposed here as a single canonical set for extension modules and downstream tooling. Backward compatibility - All additions are new methods or new protected modifiers on non-final classes. No removals, no signature changes. - The full jts-core suite (2282 tests) passes unchanged.
Adds an opt-in module that implements the OGC SFA / ISO 19125-2
extended geometry types on top of the extension hooks added to
jts-core in the prior commit.
Geometry types (in org.locationtech.jts.geom.curved)
- CircularString extends LineString
- CompoundCurve extends LineString (member structure collapsed in
this phase)
- CurvePolygon extends Polygon (rings linearised on read)
- MultiCurve extends MultiLineString
- MultiSurface extends MultiPolygon
- Triangle extends Polygon (Polygon with one 4-point ring)
- PolyhedralSurface extends MultiPolygon
- Tin extends PolyhedralSurface
`Triangle` lives in `.geom.curved` because the existing static-utility
class `org.locationtech.jts.geom.Triangle` (centroid, circumradius)
is preserved unchanged.
Readers / writers (in org.locationtech.jts.io.curved)
- CurvedWKTReader extends WKTReader, overrides
`readOtherGeometryText` to dispatch the eight new keywords. Composite
types (CompoundCurve, CurvePolygon, MultiCurve, MultiSurface) iterate
heterogeneous members via `lookAheadWord`-based peeking. CompoundCurve
also accepts the writer's flat round-trip form.
- CurvedWKTWriter extends WKTWriter; phase-1 marker. Emission already
works correctly via the core `getGeometryType().toUpperCase()` change
routed through the existing `instanceof` ladder.
Tests
- 54 round-trip tests across 8 files covering XY / XYZ / XYM / XYZM /
EMPTY / round-trip / structural-validation-deferred negative cases.
- All pass against the new module; full jts-core suite (2282 tests)
still passes unchanged.
Out of scope (deferred to later phases per the proposal)
- Validation (Triangle 4-point rule, CircularString odd point count,
CompoundCurve member connectivity, Tin triangle-only patches).
- Member-structured CompoundCurve / CurvePolygon emission (currently
collapsed to flat coordinates / linear rings).
- WKBReader / WKBWriter for SFA-MM type codes.
- Native curve-aware spatial operations (intersects, contains, area,
buffer). Curves currently fall through to their parent type's
polyline / polygon behaviour.
- copy() returning the correct subclass.
- JTSTestBuilder UI integration.
References
- Discussion: locationtech/jts#1193
- Design template: NetTopologySuite/NetTopologySuite#526
…README Addresses code-review feedback on PR locationtech#1194. All additions are still purely additive at the jts-core level. jts-curved - New CurvedGeometryFactory extends GeometryFactory with creation methods for the eight new types (createCircularString, createTriangle, etc.). Pair with CurvedWKTReader for the standard read-construct-emit flow. - New Linearizable interface (Geometry toLinear(double tolerance)) for converting curved geometries to non-curved approximations. Phase 1 returns a parent-type geometry built from the same control points; a future phase will swap in real arc densification. - Implement Linearizable on the seven curve / curve-bounded / triangulated-surface types (Triangle, PolyhedralSurface, Tin omit Linearizable but get correct copyInternal too). - Override copyInternal() on all eight types so copy() returns the correct subclass instead of degrading to the parent. - Module README: usage example, phase-1 limitations (with explicit callout for the CurvePolygon / MultiSurface inner-member round-trip degradation), and discovery rationale (explicit instantiation rather than ServiceLoader). WKTMultiSurfaceTest - testWKTRoundTripXY no longer compares the original CurvePolygon- bearing geometry directly to the round-tripped Polygon-bearing one. Polygon.isEquivalentClass is strict (LineString's is lenient, which is why MultiCurve passes the analogous test); a direct checkEqual would fail. Switched to (a) WKT-stability (write -> read -> write yields the same WKT) and (b) coordinate-equivalence via toLinear(). Comment documents the seam. jts-core - Add WKTReaderExtensionHookTest with 6 tests that exercise both the reader and writer extension hooks via dummy in-test subclasses, with no dependency on jts-curved. Confirms the seam is wired and that the promoted protected helpers are accessible across packages. - Class-level Javadoc on WKTReader now documents the extension contract and lists the protected helpers available to subclasses. Same for WKTWriter (intercept point, parameterised keyword emission, and the exposed text-emission helpers). - Per-field Javadoc on the geometryFactory and csFactory protected fields explaining their role in extension subclasses. Net change: jts-core grows by 6 tests (2288 total, was 2282). jts-curved holds 54 tests, all green. Combined: 2342 tests, BUILD SUCCESS. Discovered seam (recorded in MultiSurface test comment + README): Polygon.isEquivalentClass is strict, so a Polygon and a coord-identical CurvePolygon are not equalsExact even though LineString solves the analogous problem leniently. Phase 2 (member-tagged emission in CurvedWKTWriter) will close this gap; phase 1 documents and works around it.
Phase 4-A of the SFA / ISO 19125-2 curve work (per discussion #1193). Single-purpose change: every WKT-parsing path inside jts-app now uses the curve-aware reader and factory, so curved WKT can be pasted, loaded, saved, and round-tripped without any new UI. What now works in JTSTestBuilder - Pasting `CIRCULARSTRING(1 5, 6 2, 7 3)` (or COMPOUNDCURVE, CURVEPOLYGON, MULTICURVE, MULTISURFACE, TRIANGLE, POLYHEDRALSURFACE, TIN, with optional Z/M/ZM modifiers) into the Input A/B panels. - Loading a `.jts` test case that contains curved WKT. - Loading WKT/WKB files via File menu. - The Geometry tree shows the correct type name (CircularString, etc.). - Spatial functions fall through to the parent type's behaviour (polyline / polygon) per the phase-1 contract; explicit linearisation is available via Linearizable.toLinear(tolerance). Files touched (all jts-app) - pom.xml: add jts-curved dependency. - testbuilder/JTSTestBuilder.java: fallback geometry factory is now CurvedGeometryFactory. - testbuilder/model/TestBuilderModel.java: default geometry factory is CurvedGeometryFactory; loadGeometryText and the post-precision-model- change reload path use CurvedWKTReader / CurvedGeometryFactory. - testbuilder/GeometryInputDialog.java: input parser uses the curved reader / factory. - util/io/IOUtil.java: readWKTString uses CurvedWKTReader (this is the central WKT path, also reached via MultiFormatReader). - util/io/MultiFormatBufferedReader.java: readWKT uses CurvedWKTReader. - util/io/MultiFormatFileReader.java: readWKTFile uses CurvedWKTReader. - test/TestCase.java: initGeometry and toNullOrGeometry use CurvedWKTReader / CurvedGeometryFactory for test-case WKT parsing. What is intentionally NOT in this commit - No drawing tools for curved geometries (defer to Phase 4-B). - No CurvedShapeWriter / Bezier rendering (defer to Phase 4-B). - No GeometryType enum extension or controller mode methods. Net diff: +30 / -15 lines across 8 files. Reactor build is green (jts-core 2288 + jts-curved 54 tests pass; all other modules unchanged).
The five "testRejectsX" methods across the curved-module test suite
used the pattern
try {
reader.read(badInput);
fail("...");
} catch (Throwable e) {
// expected
}
which silently passes whether or not the reader actually rejects the
input -- JUnit 3's fail(...) throws AssertionFailedError, which is a
Throwable, which the catch block swallows. So the tests passed in CI
even though four of the five validations are not implemented yet:
- CircularString odd-point-count rule -- not validated
- CompoundCurve member-endpoint connectivity -- not validated
(silent coord-drop)
- Triangle 4-point-only ring -- not validated
- Triangle no-inner-rings -- not validated
- Tin triangle-only patches -- not validated
This contradicted the curve-awareness spec epic's "landing a sub-issue
deletes its red test" convention: a deceptive green test cannot serve
as a live progress meter.
Rewrites each spurious "rejects" test as an explicit acceptance test
naming the deferred OGC SFA rule and pointing at the spec-epic sub-
issue tag (VAL-CS, VAL-CC, VAL-T, VAL-TIN). The assertion captures
the actual current behaviour (e.g. lossy 3-coord output for the
disconnected COMPOUNDCURVE case), so when the validation phase lands
the assertion flips red and the maintainer is prompted to convert
the test back to an explicit expectThrows(ParseException) -- or
delete it per the epic convention.
The two unclosed-ring tests (Triangle / CurvePolygon) DO have real
validation behind them today, via LinearRing's IllegalArgumentException
on non-closed coordinate sequences. Their catch blocks are narrowed
from Throwable to IllegalArgumentException so a future regression is
caught loudly instead of silently.
No production-code change -- this commit is purely test integrity.
54 curved-module tests still green.
Assisted-by: Claude (Opus-4.7)
Signed-off-by: Jeroen Bloemscheer <jeroen@jeroentechsolutions.uk>
CurvedWKTReader previously rolled its own static matchesType helper
that duplicated the Z/M/ZM suffix logic already present in
WKTReader.isTypeName. Two small downsides:
- The hand-rolled matcher returns false on an unrecognised suffix
(e.g. "CIRCULARSTRINGFOO"), so the input falls through to
super.readOtherGeometryText which throws the generic
"Unknown geometry type". isTypeName instead throws the precise
"Invalid dimension modifiers: CIRCULARSTRINGFOO" -- strictly
better diagnostics for the same dispatch path.
- Any future readOtherGeometryText extender (a downstream module
for 3-D solid types, GeoSPARQL flavours, etc.) would need to
reproduce the modifier logic from scratch, with the usual risk
of drift if the core list of recognised modifiers ever changes.
Change: promote WKTReader.isTypeName from private to protected (with
Javadoc clarifying the contract for subclasses), delete the duplicate
matchesType helper in CurvedWKTReader, and route all eight
keyword-dispatch branches through isTypeName.
Behaviour is the same for valid input; invalid dimension modifiers
now surface as a precise ParseException instead of a generic one.
2288 jts-core tests + 54 jts-curved tests still green.
Assisted-by: Claude (Opus-4.7)
Signed-off-by: Jeroen Bloemscheer <jeroen@jeroentechsolutions.uk>
readSurfaceMember dispatches on `instanceof Polygon`, which means a tagged TRIANGLE keyword (Triangle extends Polygon) is a legal MULTISURFACE member -- but no existing test exercised that path. The dispatch is a contract surface for downstream extenders adding new Polygon subclasses; lock it in. Assisted-by: Claude (Opus-4.7) Signed-off-by: Jeroen Bloemscheer <jeroen@jeroentechsolutions.uk>
Phase 4-A's test plan marked the manual JTSTestBuilder smoke test as "defer to maintainer review". Land a concrete recipe so a reviewer (or a future contributor verifying a regression) can paste a row, click Load, and check the geometry-tree label without having to guess at the expected outcome. The table covers all eight new types plus a Z/M/ZM dimension variant and ends with the round-trip-via-Save step that exercises the .jts file path through CurvedWKTReader / CurvedGeometryFactory. The toLinear escape hatch is referenced as a programmatic API, flagged as still-deferred-to-Phase-4-B for the UI panel binding. Assisted-by: Claude (Opus-4.7) Signed-off-by: Jeroen Bloemscheer <jeroen@jeroentechsolutions.uk>
…ests Of the 49 TAGs in the SFA Curve Awareness epic (issue locationtech#1195), F-CP (structural CurvePolygon) carries the heaviest dovetailing load: - It is the hard Phase-1 prerequisite for every Phase-2 TAG that needs to talk about a curved boundary, area, or validity (B-CP, M-AREA-CP, V-CP). - It carries the headline risk #1 in epic §7: a structural CurvePolygon whose shell is a CompoundCurve cannot also satisfy Polygon's legacy LinearRing-typed accessor without an explicit design choice. - The existing Phase-1 stand-in in jts-curved silently linearises CurvePolygon rings on read; a future structural form must coexist with or replace that path. - It depends on (and is unblocked by) the just-landed feature/sfa-curve-compoundcurve-members work, so the precondition is real, not aspirational. The single test_F_CP_* method in CurveAwarenessSpecTest documents the headline gap. This spike adds: modules/curved/src/test/java/org/locationtech/jts/spec/curveawareness/ CurvePolygonStructuralSpec.java 8 red tests covering the F-CP sub-surface: - FCP-S shell is exposed as a Curve (Compound or Circular), not a LinearRing - FCP-MEM CompoundCurve shell preserves member subtypes - FCP-H interior rings (holes) are Curves per spec - FCP-CP copyInternal preserves shell + holes as Curves - FCP-TL toLinear(tolerance) walks shell and each hole - FCP-WKT WKT round-trip preserves the structural ring tag - FCP-DOVE the legacy Polygon API contract -- design decision point, see SPEC_F_CP.md modules/curved/SPEC_F_CP.md Captures the three live options for the FCP-DOVE decision (legacy fallback / widen return / fail-fast) as a branching table with trade-offs, where we lean today, and what's deferred. Lays out the smallest concrete next step: a one-line maintainer ack on the option, then a separate `arch:` commit recording the choice, then the implementation PR. modules/curved/pom.xml Adds the Surefire exclude on **/spec/curveawareness/*.java per epic §11 -- the spec class is intentionally red and runs on demand via `mvn -pl modules/curved test -Dtest=CurvePolygonStructuralSpec`. Default reactor build stays green (55 jts-curved tests, all green, spec class excluded). Posture verified: - default `mvn test -pl modules/curved` => 55 / 55 green - on-demand `-Dtest=CurvePolygonStructuralSpec` => 8 run, 8 failed, each fail() naming its sub-TAG so the message stream is a live progress meter - checkstyle `mvn install -pl modules/curved` => BUILD SUCCESS Assisted-by: Claude (Opus-4.7) Signed-off-by: Jeroen Bloemscheer <jeroen@jeroentechsolutions.uk>
Prototype Option A from SPEC_F_CP.md: keep Polygon.getExteriorRing()
returning a LinearRing of the linearised chord coords (legacy callers
keep working unchanged), and add a new getExteriorCurve() accessor
that returns the structural shell (CompoundCurve / CircularString /
LineString / LinearRing).
Diff: +60 / -8 lines across three files.
- CurvePolygon: new private LineString structuralShell field, new
(LineString, LinearRing[], GeometryFactory) constructor that
derives the legacy LinearRing via Linearizable.toLinear(0.0) and
new getExteriorCurve() accessor. copyInternal preserves the
structural shell.
- CurvedWKTReader.readCurvePolygonText: captures the first member as
a structural LineString and feeds the structural-aware
constructor; subsequent members still linearise to LinearRing
holes (FCP-H is a separate sub-issue).
- CurvePolygonStructuralSpec: helper structuralShellOf now calls
getExteriorCurve(); FCP-DOVE flips from "fail with design question"
to a positive assertion that the Option-A accessor works.
Findings (run on this branch):
- 55 jts-curved Phase-1 tests stay green -- Option A is fully
backward-compatible at the curved-module surface.
- 4 of 8 FCP red tests flip green (FCP-S, FCP-S-singleArc, FCP-CP,
FCP-DOVE) -- proof that the structural-shell mechanism, the
copyInternal propagation, and the design-decision accessor all
work.
- 4 reds remain (FCP-MEM, FCP-H, FCP-WKT, FCP-TL). All four track
independent sub-issues that any of A / B / C would have to address
separately:
FCP-MEM CompoundCurve member preservation -- precondition
lives on feature/sfa-curve-compoundcurve-members
FCP-H interior-ring symmetry (parallel getInteriorCurveN
accessor)
FCP-WKT writer must emit COMPOUNDCURVE tag inside body
FCP-TL toLinear walking + densification -- depends on
feature/sfa-curve-toLinear-densification
- Full reactor build with checkstyle: BUILD SUCCESS.
Discovery for the issue: Option A is implementable in ~60 LOC of
additive change, with no breaking impact on existing jts-curved tests
or jts-core. The remaining open question for A is the default
tolerance source for the linearised getExteriorRing() view (today
toLinear(0.0) ignores tolerance; a real implementation would
parameterise on PrecisionModel scale or CurvedGeometryFactory state).
That decision sits inside the implementation PR, not in the
A-vs-B-vs-C choice.
Assisted-by: Claude (Opus-4.7)
Signed-off-by: Jeroen Bloemscheer <jeroen@jeroentechsolutions.uk>
- CurvePolygon now accepts/roundtrips/preserves LineString (incl. CircularString/CompoundCurve) shells and holes - Added getExteriorCurve / getInteriorCurveN; legacy get*Ring return densified LinearRing per Option A - Reader collects structural rings for CURVEPOLYGON - Writer overrides appendOther to emit tagged curved rings inside CURVEPOLYGON bodies - Factory extended with structural createCurvePolygon(LineString, LineString[]) - copyInternal / toLinear(tol) walk and preserve the structural curves - Removed F-CP (and F-MC/F-MS as they were already structurally supported) red tests from CurveAwarenessSpecTest - Cleaned FCP tests from structural spike spec; kept DOVE as contract test (now passes) - References locationtech#1195 Assisted-by: xAI Grok
…ec tests for proper curved ring syntax - Move normalization expression into super() arg list so super is first statement (flexible ctors not available under -source 8). - Fix several CURVEPOLYGON ((TAG... WKTs in CurveAwarenessSpecTest to use canonical CURVEPOLYGON (TAG... or COMPOUNDCURVE ( for compound rings. Prevents read() errors so higher TAGs can execute. - Verified: curved normal tests 55/55 green; structural DOVE test green; main spec now 46/46 reaching their fail() (no more parse crashes for F-CP related). Part of locationtech#1195 F-CP landing.
…(), normalize() in CurvePolygon to preserve structural curves (HIGH severity from review) - reverseInternal now reverses the structural* members and reconstructs via structural ctor (prevents silent degradation to LinearRings via base impl + legacy factory path). - Public reverse() overridden to return CurvePolygon (like copy). - normalize() calls super (to keep legacy views normalized per Polygon contract / Option A) but leaves structural curves as source-of-truth (avoids losing arc identity). - Matches reviewer suggestion in /tmp/grok-review-d5a00063.md item 1. - Still defers deeper arc-orientation normalization. Part of F-CP for locationtech#1195. Review artifacts preserved locally.
… and remove per-ring Z/M qualifiers; doc+test: explicit note+test_FCP_EQ for structural curves not affecting equalsExact (EPIC §7 / R-EQ); test: add adversarial CurveRefRunner/CounterexampleHunter/CurveAdversarialTest + arc vectors (inspired by PR#1197 RocqRefRunner for M-LEN-*); test: enhance WKTCurvePolygonTest for 3D/compound roundtrips
…ith seam ID (BoundaryOp/Polygon/MultiPoly/CurvePoly seams); feat: simplest CurvePolygon.getBoundary override using structuralRings (green, don't integrate: meter fail stays); refactor: extract structuralRings helper (DRY copy/reverse too), allLinearRings soundness for linear-degen CP, better comments/empty; verification test added to CurvePolygonStructuralSpec (passes while meter still red per epic); branch: feature/sfa-curve-B-CP-rgr
…ith seam ID (MultiPolygon collection, post-B-CP boundary types, curved MS return); feat: simplest MultiSurface.getBoundary override (green, always MC for curved members, leave meter TAG fail); refactor: soundness for pure-linear MS (return plain MLS), anyCurved detection (incl. CP members + non-LR children), better comments; verification test in structural spec (passes while meter red). branch: feature/sfa-curve-B-MS-rgr
…rst-batch arc/buffer/precision issues; more cases for M-LEN-* TAGs)
…LEN_CS with seam ID (LineString.getLength/Length.ofLine on controls, stride-2 walk, CurveRefRunner exact); feat: CircularString.getLength override + exact helper (green, delegates to r*theta per arc; meter TAG fail untouched); refactor: update hunter test + class doc (now 0 deviations for CS, as expected post-fix); verification via existing load/validate + adversarial. branch: feature/sfa-curve-M-LEN-CS-rgr
…-CS landed per RGR)
…pattern: add Result+run to CurveRefRunner (modeled on core); use in load test to assert geom.getLength() sound on vectors; add boundary hardening test using orient refSign on curve controls from boundaries; update near-flat test comments/asserts. 5 adversarial tests now pass.
test: extend red test_M_LEN_CC javadoc + update live progress header
feat: CompoundCurve.getLength override (sums member.getLength(); delegates to
CircularString analytical for arcs + LineString for segments). Includes
detailed RGR seam doc in javadoc modeled on M-LEN-CS.
arch: integrate prerequisite structural CompoundCurve (members, getNumCurves,
getCurveN, copy/toLinear member-aware) + CurvedWKT* updates for member
emission (tagged CIRCULARSTRING members in COMPOUNDCURVE output) from
compoundcurve-members spike + F-CP-era writer extensions. Resolves to
current reader/writer state (CurvePolygon ring support + structured CC).
refactor: writer now emits proper mixed-member COMPOUNDCURVE WKT when
structural members present (advances F-MC round-tripping).
Verification: mvn ... -Dtest=...#test_M_LEN_CC... now reports actual == expected
(25.70796...) before the convention fail("TAG:...") line. Meter method kept
per §5 (delete only on ship).
branch: feature/sfa-curve-M-LEN-CC-rgr
Assisted-by: xAI Grok (in the loop for picking TAG + executing the RGR)
- Update current branch, Phase 2 status, next steps, files touched. - M-LEN-CC now joins M-LEN-CS as green (no hardening in this pass; vectors already exist for future). Follows the RGR for the TAG on feature/sfa-curve-M-LEN-CC-rgr.
…urveMembersTest Exercises M-LEN-CC via both CompoundCurve and Geometry paths with the mixed line+arc case from the spec meter. Passes cleanly (actual == expected). Complements the red marker in CurveAwarenessSpecTest per RGR style (verification lives next to the TAG impl; meter fail untouched until ship). On branch feature/sfa-curve-M-LEN-CC-rgr
test: expand red test_B_CC javadoc + update live progress header + EPIC_PROGRESS
feat: CompoundCurve.getBoundary() explicit guard override (B-CC).
Delegates to super (BoundaryOp lineal rules) but makes the contract
first-class for the structural CompoundCurve (open -> MultiPoint of
member-derived endpoints; closed -> empty). Full RGR seam doc in javadoc.
test: add green verification testBoundaryForOpenAndClosedCompoundCurves
(open case with coord asserts + closed case -> empty; exercises the
structural endpoints from members).
The pre-existing assert in the meter now runs against the guarded impl.
Verification test is green. Meter fail("TAG: B-CC...") left per convention.
Builds on M-LEN-CC + structural members work.
branch: feature/sfa-curve-B-CC-rgr
Assisted-by: xAI Grok (TAG selection + RGR execution loop)
For completeness with the CompoundCurve guard and the seam comments
("curved *lineals* (CC, CS) get line-boundary logic").
No behaviour change (delegates); just makes the contract explicit on the
curved subtype and robust for future.
On feature/sfa-curve-B-CC-rgr (B-CC TAG).
…(via the symmetric guard) Also exercises a (degenerate) closed CS case. refactor: the CS getBoundary guard was added in prior commit on this branch. branch: feature/sfa-curve-B-CC-rgr
feat: CurvePolygon.isValid() override for arc-aware validity (V-CP).
- Uses structural rings (post F-CP).
- Ring validity: closed + isSimple() forwarded to CircularString/CompoundCurve
(new analytical arc self-intersect using circle intersections + sweep).
- Orientation: signed sector area (shoelace + r^2/2*(theta-sin) corrections)
for shell/holes consistency + explicit CCW check on view.
- Holes in shell: representative point contained in densified shell (practical;
self+orient are the analytical core; full arc PIP for later relate TAGs).
- Also runs super.isValid() on the linear view for other rules.
Added helpers in CircularString (computeCircle, arcsIntersectProper, isSimple)
and CompoundCurve (isSimple delegating + cross).
test: V-CP verifications in CurvePolygonStructuralSpec (valid CP, invalid orient,
self-intersect exercise via isSimple on curved ring).
spec: updated red test javadoc + progress headers.
doc: EPIC_PROGRESS_OVERVIEW.md updated for V-CP landed.
Builds on B-CC/M-LEN-CC isSimple reuse for rings.
branch: feature/sfa-curve-V-CP-rgr
(Per epic Phase 2; meter fail left per §5.)
…liable - Enhanced CircularString.isSimple() with repeated control point check (catches the classic V-CS "loops back over itself" case at interior revisit points like (0,0), in addition to geometric arc-arc crosses). - Similar repeated-point guard in CompoundCurve.isSimple(). - Added roundKey helper for stable float keys. - Removed debug println; strengthened the self-intersect verification test to assertFalse on the overlapping CS (now passes thanks to the point-revisit logic). - Verified: all 3 V-CP tests green, full curved tests 69/69 green. - This makes the analytical checks in CurvePolygon.isValid() (for V-CP) more effective for the documented self-overlap scenarios. branch: feature/sfa-curve-V-CP-rgr
Live spec and red-test meter for the SFA Curve Awareness epic (locationtech#1195). Documents all TAGs including V-CP (IsValidOp for CurvePolygon with arc self-intersection, sector orientation, arc-aware hole containment) and V-CS (IsSimpleOp for CircularString/CompoundCurve). Created to live in the JTS tree (grootstebozewolf/jts) per instructions. Companion to EPIC_PROGRESS_OVERVIEW.md and CurveAwarenessSpecTest.java. References proofs-side triage and the red-test convention (delete methods on ship; meter count as progress).
The V-CP branch introduced instanceof CircularString usage in appendCompoundCurveTaggedText without the import. This was causing compilation failure on the branch. Small build hygiene commit to make the tree usable post-checkout and EPIC doc addition.
…ening - Extend CurveCounterexampleHunter with ValiditySimplicityMismatch, generators (selfOverlappingArc, selfIntersectingCurvePolygon, etc.) and huntIsSimple/ huntIsValid for nice adversarial counterexamples. - New tests in CurveAdversarialTest exercising the V-CS/V-CP hunters (prints examples like self-overlap !isSimple and self-intersect shell !isValid). - Harden CompoundCurve.isSimple() non-adjacent cross checks to use arc-aware CircularString.arcsIntersectProper (for arc members) in addition to linear intersects (directly supports V-CS/V-CP). - Update CurveAwarenessSpecTest.java and EPIC_SFA_CURVE_AWARENESS.md comments for PRC-SN (and V-CP) to cite the fresh proofs oracle artifact (run 26928081635/art 7402195928 for JTS#979 + power-of-two hotpixel + curve snap soundness). - Add curve_snap_vectors.txt to src/test/resources (refreshed header with this artifact; was only in target before) for PRC-SN hardening (used by V-CP under precision). - Ties into existing V-CP isValid (sector area, isSimple on rings, hole checks) and V-CS isSimple (arc cross + revisit) on the branch. - Uses the sharp oracle-bin-linux for 979/precision/curve-snap/arc predicates. Hunters surface cases for further hardening. See also: NetTopologySuite.Proofs artifact for exact Q CURVE_SNAP_DECISION, HOLE_PRECISION_AUDIT etc. References JTS#1195 epic.
Contributor
Author
|
Closing per review feedback. The PR accumulated the full epic history (33 commits stacked on unmerged base from #1194/#1198). The core V-CP/V-CS delta (last ~3 commits: arc-aware isSimple on CompoundCurve using arcsIntersectProper, CurvePolygon validity hardening, counterexample hunter in CurveAdversarialTest) is solid and the right shape for a focused PR. Hunter finding 0 mismatches means the impl is clean. Will keep the feature/sfa-curve-V-CP-rgr branch locally for development. Will reopen a clean, self-contained PR (only the V delta, no foundation commits, no meter language) once the base PRs (hooks, jts-curved, F-CP etc.) are merged to locationtech:master. New pattern: each TAG gets its own PR branched from the previously merged upstream commit. One TAG, one PR, green and reviewable on its own. Red meter convention dropped in favor of this decomposition. See discussion in this thread for details. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements and hardens V-CP (arc-aware
IsValidOp/CurvePolygon.isValid()) and V-CS (IsSimpleOpforCircularString/CompoundCurve) on top of the structural F-CP work.CurveCounterexampleHunterwithValiditySimplicityMismatch, dedicated generators (self-overlapping arcs, self-intersecting CurvePolygons, simple good cases) andhuntIsSimple/huntIsValid. Used to surface adversarial cases for hardening (e.g. overlap detection via arc crosses + point revisits, sector orientation, hole checks).testHunterForVCSNonSimpleCounterexamplesandtestHunterForVCPValidityCounterexamplesinCurveAdversarialTest(modeled on arc-length + RocqRefRunner pattern). Always emit explicit "nice" examples from the spec (self-overlap returns!isSimple, bad shells!isValid).CompoundCurve.isSimple()now uses arc-awareCircularString.arcsIntersectProperfor non-adjacent arc members (instead of only linearintersects).CurvePolygon.isValid()+CircularString.isSimple()(and Compound) already provide the analytical parts (self-intersect, sector signed area corrections, etc.).CurveAwarenessSpecTest.javaandEPIC_SFA_CURVE_AWARENESS.md(PRC-SN section) to cite the fresh proofs oracle artifact for supporting precision/snap cases relevant to V-CP.curve_snap_vectors.txt(now insrc/test/resources+ target) with header referencing run 26928081635 / artifact 7402195928 (JTS#979 + power-of-two hot pixels + CURVE_SNAP_DECISION + HOLE_* modes).Related
feature/sfa-curve-V-CP-rgr), and the red-test meter convention inCurveAwarenessSpecTest(red markers kept; delete on full ship).CurvePolygonStructuralSpecalready has some green V-CP verifications; these add the hunter/adversarial layer.Testing
mvn -pl modules/curved test -Dtest=CurveAdversarialTest,CurvePolygonStructuralSpec(all pass, hunters exercised, nice examples printed).See commit 3f8180a for details.