big push with a lot of changes, features, and pip preparation

[oscarfasanchez]

This pull request prepares the vorflow project for its first public release candidate (0.1.0rc1) with a focus on packaging, testing, and documentation improvements. It adds a changelog, a detailed project roadmap, and refines the README with clearer installation and usage instructions. The CI pipeline is enhanced with minimum dependency testing, linting, and coverage reporting. Additionally, a new workflow automates publishing release candidates to TestPyPI.

Release preparation and documentation:

• Added a CHANGELOG.md following Keep a Changelog and Semantic Versioning, documenting all notable changes for 0.1.0rc1.
• Added a comprehensive ROADMAP.md and milestone documents under docs/roadmap/, outlining ported features, decisions, and verification steps for the release. [1] [2] [3]
• Improved the README.md with clearer installation instructions (including pip and development modes), usage examples, and explanations for mesh gradation and coordinate systems. [1] [2]

Continuous integration and testing enhancements:

• Updated .github/workflows/python-app.yml to:
  • Add a lint job using Ruff for code style checks.
  • Expand test coverage to Python 3.10, 3.11, and 3.12.
  • Run tests with coverage reporting.
  • Add a minimum-dependencies job to verify the package works with minimum supported versions of core dependencies. [1] [2]

Packaging and distribution:

• Added .github/workflows/testpypi.yml to automate building and publishing release candidates (tags matching v*rc*) to TestPyPI, including build verification, metadata checks, and a smoke test of the built wheel.
• Added a MANIFEST.in to exclude development artifacts and documentation from source distributions.

These changes collectively ensure robust packaging, testing, and documentation for the first public release candidate of vorflow.

[rhugman]

Thanks for this — the feature work here is substantial and the test suite going from ~10 to 204 tests is a big step up. I ran it locally: ruff check src tests scripts is clean and all 204 pass in ~4s.

Given the size (13k additions, 49 files), I've split my feedback into things I think need resolving before merge and things that can follow up.

---

Needs resolving before merge

1. Release metadata points at the fork

pyproject.toml declares:

Repository = "https://github.com/oscarfasanchez/vorflow_os"
Issues     = "https://github.com/oscarfasanchez/vorflow_os/issues"
Changelog  = "https://github.com/oscarfasanchez/vorflow_os/blob/main/CHANGELOG.md"

If we merge and tag from here, we'd publish a package named vorflow whose every URL points at a different repository — bug reports would land in the wrong tracker. Same for the 5 README links and the CHANGELOG compare links.

This also affects TestPyPI trusted publishing, which is configured per-repository, so testpypi.yml won't authenticate as-is.

Three files assert these values, so pyproject.toml can't be fixed alone — CI will fail the corrected metadata:

• scripts/check_dist.py:24-28 (EXPECTED_URLS)
• tests/test_check_dist.py:35-42
• tests/test_release_metadata.py:24-30

Happy to talk through how you'd like attribution handled — you've clearly done the bulk of the work here and I want that reflected properly. Just need the canonical URLs to be this repo.

2. ConceptualMesh.generate() mutates its own inputs

blueprint.py:694-698:

embedded_polys   = [p for p in self.raw_polygons if bool(p.get("embed", True))]
field_only_polys = [p for p in self.raw_polygons if not bool(p.get("embed", True))]
self.raw_polygons = embedded_polys   # field-only polygons dropped from state

_clip_features_to_domain() reassigns self.raw_lines / self.raw_points the same way. Calling generate() twice silently loses every embed=False refinement polygon:

run1: polys=2  zones=[1, 2]
run2: polys=1  zones=[1]

No warning — just a coarser mesh. This matters because generate() now takes a connectivity_tolerance= override, which invites re-calling it to tune, and the examples are notebooks where re-running a cell is routine.

Fix is to work on a local copy and thread the partition through _resolve_overlaps rather than via instance state. Worth a regression test asserting two calls give identical frames.

3. MeshGenerator.__init__ reconfigures global logging

engine.py:145, in the constructor:

set_verbosity(verbosity)   # default verbosity=0 -> WARNING on the "vorflow" logger

Constructing MeshGenerator(background_lc=5.0) with defaults silences all vorflow logging process-wide, including ConceptualMesh's progress output. Verified — after that line a subsequent cm.generate() prints nothing.

Two things compound: object construction mutating global logging config, and MeshGenerator's default (0) disagreeing with the package default set in __init__.py:12 (set_verbosity(1)), so the silencing path is the common one.

Suggestion: keep verbosity per-instance for the gmsh General.Verbosity option and the if self.verbosity > 1 guards, and let vorflow.set_verbosity() be the only thing touching the logger.

This one also hides the next two items.

---

Correctness follow-ups

Dropped features are reported at INFO. _clip_features_to_domain() removes points outside the domain with only logger.info. Verified: a well 1 unit outside the boundary disappears silently. For a groundwater tool, discarding a well or observation point deserves warnings.warn naming the affected point_id / line_id. Note _enforce_connectivity snaps points onto the boundary just before, so a point landing a float-epsilon outside can be dropped by covers.

Failure message is invisible. engine.py:2953 uses logger.info(f"Mesh Generation Failed: {e}") — below the default threshold. logger.exception(...) would be better. It re-raises, so nothing is lost permanently.

background_lc is advertised optional but is mandatory. __init__ keeps background_lc=None; _setup_fields raises ValueError at line 2181, after geometry transfer, fragmentation and embedding have run. Validate in __init__ so it fails fast, or make it required positional.

---

CI / release

Action versions look ahead of what's published, and are inconsistent within one file. In python-app.yml, lint and test use checkout@v4 / setup-python@v5 while minimum-dependencies uses checkout@v7 / setup-python@v6. testpypi.yml uses checkout@v7, setup-python@v6, upload-artifact@v7, download-artifact@v8. I couldn't reach the API to confirm the published majors from my environment — could you double-check? An unresolvable action fails the job before any step runs.

Smoke test pins the version it asserts. testpypi.yml:41 hard-codes assert vorflow.__version__ == '0.1.0rc1', but the workflow triggers on v*rc*, so v0.1.0rc2 would fail. check_dist.py already has version_from_tag() for this — derive it from ${{ github.ref_name }}.

python-app.yml no longer runs on pushes to develop. on.push.branches is ["main", "master"]. PRs still trigger, so it's a gap rather than a break, but merges to develop won't be tested.

Smaller: scripts/check_dist.py imports packaging, which isn't declared anywhere (resolves transitively via pytest/build) — add it to dev. README advertises pip install vorflow, but only TestPyPI is wired up. pip install -e .[dev] needs quoting for zsh.

---

Code quality

engine.py is now 2,955 lines, and _add_geometry spans 725–1778 — a single ~1,050-line method with roughly 20 nested closures. I didn't find anything wrong in it, but that's partly the problem: it's where the trickiest geometry lives and it's very hard to review or unit-test at that size. The structured-buffer machinery (planning, trimming, crossing arbitration, transfinite application) reads as a coherent unit that would sit well in its own module, the way you extracted fields.py.

fields.py is nicely done — good docstrings, clean validation helpers, honest documentation of the internal-vs-public split. Two small things:

• ThresholdField.__init__ does bare float() casts while ExponentialField and GeometricGrowthField go through _positive_finite. ThresholdField is public and in __all__, so it should validate too. Related: self.size_max if self.size_max else background_lc (line 263) treats size_max=0.0 as unset — is not None is safer.
• AutoLinearField / AutoExponentialField ship as deprecated aliases, but they were never released (they only exist in the still-open adding flexible fields to tweak and optimize cells size  #11). Since 0.1.0rc1 is the first release, I'd just drop them.

Field grouping keys on the hash rather than the object — engine.py:2340 uses key = (hash(field), feature_lc). MeshField already defines __eq__ and __hash__, so key = (field, feature_lc) is simpler and collision-proof. Robustness rather than a live bug.

inset_mirror boundary centering is quadratic. tessellator.py:192 calls boundary.distance(point) in a Python loop over all nodes, and _ring_angle_at_point then rescans every ring coordinate per boundary node — O(B × R), where a densified boundary makes R ≈ B. You correctly moved the nearest-neighbour spacing to a cKDTree; these two didn't get the same treatment. shapely>=2.0 is already a floor, so shapely.distance(boundary, points_array) vectorises the first, and hashing ring vertices once removes the second. Opt-in mode so not urgent, but it'll bite on a 10⁵-node model.

docs/superpowers/ — ~1,600 lines of planning/spec scratch that duplicate pyproject.toml content verbatim and will rot immediately. I'd drop those; ROADMAP.md and docs/roadmap/* are genuinely useful, keep those.

---

Tests

Coverage is much better. test_buffer.py, test_point_tracking.py and test_quality_metrics.py are all substantial, and marking the gmsh-heavy modules slow is good practice (nothing in CI deselects them yet, so it's declarative for now).

Gaps:

• No idempotency test for generate() — that's how item 2 survived.
• No test that dropped out-of-domain features get reported.
• test_release_metadata.py asserts exact author lists, the exact build-system.requires string, and exact dependency pins. That restates pyproject.toml rather than testing anything, and will fail on any legitimate contributor or dependency change. The valuable parts are the __version__ fallback check and test_basic_usage_script_runs_from_a_clean_directory — I'd trim the rest.

---

One process thought

This functionally supersedes #9 (simplify_tolerance) and #11 (the MeshField framework) — both are present here. Worth closing those as absorbed.

Would you consider splitting this into two PRs? The release-engineering half (packaging, CI, changelog, MANIFEST) is low-risk and easy to verify, and could land quickly. The meshing-feature half needs more scrutiny and would benefit from being reviewed on its own. The two halves share no code.

[rhugman]

Second pass — full read of the implementation (engine.py, blueprint.py, tessellator.py, utils.py, fields.py, ~5,500 lines). Release plumbing is covered in my earlier comment; this is only the meshing code.

Everything below is code this PR introduces. Four things I found are pre-existing on develop and I've raised them separately so they don't block here: #13, #14, #15, #16.

Items marked verified I reproduced locally.

---

Correctness

Point deduplication is asymmetric, and the docstring describes different behaviour — verified

blueprint.py:426-476. The docstring says:

points that are within a specified tolerance of each other are merged, and only the point with the finest (smallest) resolution is kept

What actually happens is that a point with simplify_tolerance=None is kept unconditionally (line 449), so whether a pair merges depends on which of the two carries the tolerance, not on the distance between them:

# A: fine point carries the tolerance
cm.add_point(Point(0,0), 'fine',   resolution=1, simplify_tolerance=10)
cm.add_point(Point(5,0), 'coarse', resolution=2)
# -> ['fine', 'coarse']   both kept

# B: coarse point carries the tolerance
cm.add_point(Point(0,0), 'fine',   resolution=1)
cm.add_point(Point(5,0), 'coarse', resolution=2, simplify_tolerance=10)
# -> ['fine']             merged

Same two positions, same separation, same tolerance value, opposite outcome. Case A also contradicts the docstring directly. A symmetric rule — merge when the distance is under either point's tolerance, keep the finer — would match what's documented.

Failed embeddings are silent

engine.py:2603-2693. Every failure path in embed_entity (skip_bbox, skip_no_cand, skip_no_match, failed, inside_failed) increments a counter that ends up in self.diagnostics['embedding']. Nothing warns below verbosity >= 2.

If a point feature fails to embed, there is no mesh node at that location, so no Voronoi cell centred on it. For a well or an observation point that is a silent correctness failure — the user has no signal that the feature didn't make it into the grid, and the grid looks perfectly fine.

embed_entity doesn't currently receive the feature id, so it can't name what was lost. Threading the id through and emitting a single warnings.warn listing affected features at the end of _embed_features would fix it.

Orphan-surface recovery has no distance guard

engine.py:1744-1771. When the fragment map drops a 2D entity, the orphan is re-attached to the embedded polygon whose geometry covers its centre — good. But if nothing covers it:

if owner is None and embedded_polys:
    owner = min(embedded_polys, key=lambda item: item[1].distance(center))[0]

it attaches to the nearest polygon at any distance, reported only at INFO. That surface then enters _meshed_surface_tags, so its nodes become Voronoi generators. A guard (reject beyond a few background_lc) plus a warning would make the failure visible rather than silently distorting the grid.

Coordinate remapping uses buckets rather than a tolerance

engine.py:519 and 631-676. Both remap paths key on rounded coordinates:

coord_key = (round(bb[0], 6), round(bb[1], 6), round(bb[2], 6))       # dedup
_HEAL_ROUND = 4                                                        # heal

round() buckets rather than tolerates. The heal path comments that it absorbs "~1e-6 coordinate drift", but drift near a bucket edge flips the key, the lookup misses, and the tag is pruned — the feature silently loses its Gmsh tags and therefore its size field, counted only in an INFO line. _alive_pts_by_coord[coord_key] = t is also last-writer-wins, so two survivors landing in one bucket lose one.

A cKDTree query with an explicit tolerance would be both correct and simpler. Both constants are also in CRS units with no documentation of that dependency.

heal_shapes defaults to False, so this is opt-in — but _dedup_and_remap_fragment_map always runs.

Minor: the comment at line 519 calls round(x, 6) "~nm precision"; in metres it's µm.

push_ring_vertices_off_strips deforms the meshed domain only

engine.py:1428-1468 moves zone ring vertices that fall inside a strip onto the strip boundary — up to half a strip width. But generate() stores self.zones_gdf = clean_polys (the originals), and VoronoiTessellator._domain_geometry() clips to cm.clean_polygons (also the originals).

So where a strip reaches the domain edge, the meshed surface and the clipping domain no longer coincide. Worth checking whether this produces slivers or lost area at strip/boundary junctions.

The docstring says the move is "collinear when the ring crosses the strip straight" — true for a straight crossing, but a vertex near a strip corner projects sideways to the nearest boundary point, which isn't collinear.

---

API changes

add_polygon silently dropped four public keyword arguments

mesh_refinement, border_density, dist_max_in and dist_max_out are all gone. Existing code calling any of them now raises TypeError. The CHANGELOG has no Deprecated or Removed section and doesn't mention them.

background_lc also became mandatory (_setup_fields:2181 raises), and ConceptualMesh(crs=...) changed its default from "EPSG:4326" to None. Both are defensible, but they belong in the changelog — especially since the file declares Keep a Changelog and SemVer.

Validation is inconsistent between the three add_* methods

add_polygon validates resolution > 0 and dist_min/dist_max >= 0. add_line and add_point validate neither, so:

cm.add_line(geom, "r", resolution=-5)   # accepted

positive_number() later returns None for it and feature_lc() silently substitutes background_lc. straddle_width goes the same way. A shared _validate_feature_params() helper would make the three consistent.

Some validation happens at mesh time rather than input time

quad_buffer_thickness raises ValueError from engine.py:846 — roughly 120 lines into _add_geometry, long after the add_line() call that supplied the bad value. add_line already checks the same thing at line 302, so the engine copy is a second, later-firing check on a value that can't arrive invalid through the public API.

Related: feature_lc() falls back to a literal 10.0 (line 838) when both lc and background_lc are missing. Unreachable now that background_lc is required, but silently wrong if it ever is reached.

---

Performance

These are all Python-level loops on the diagnostics and features this PR headlines, so they'll be hit on real models.

build_connectivity (utils.py:126-129) loops per neighbour pair with two df.loc[] lookups each. Pairs scale as ~3N, so a 100k-cell grid is ~300k iterations of pandas scalar indexing plus a shapely intersection. Vectorising with shapely.intersection over arrays and positional numpy access instead of .loc would be a large win.

push_ring_vertices_off_strips (engine.py:1443-1455) calls strip.contains(point) per vertex per strip. A densified domain ring is thousands of vertices. blueprint.py already imports shapely.prepared.prep — preparing each strip once would help a lot.

inset_mirror boundary centering (tessellator.py:192) evaluates boundary.distance(point) in a Python loop over all nodes, then _ring_angle_at_point rescans every ring coordinate for each boundary node — O(B²) once the boundary is densified. You already moved the nearest-neighbour spacing to a cKDTree; these two didn't get the same treatment. shapely>=2.0 is a declared floor, so shapely.distance(boundary, points_array) vectorises the first, and hashing the ring vertices once removes the second.

_apply_structured_buffer_meshing (engine.py:2103-2123) tries every recorded corner set against every surface. create_line_structured_buffer already returns (key, strip_info) per surface, but that pairing is flattened into spec['strips'] and discarded. Beyond the unnecessary O(n²), a surface can match a different part's corner set within tol and get the wrong transfinite structure. Keeping the per-surface pairing is both faster and correct.

record_quad_buffer_crossings (engine.py:1286) sizes the refinement Ball from the bounding-box diagonal of the crossing region:

part_radius = 0.5 * math.hypot(maxx - minx, maxy - miny)

Two near-parallel strips overlapping along a long stretch give a long thin intersection whose bbox diagonal is roughly its length, so the Ball pins that entire radius at min(lc). Worth bounding by the crossing width instead.

---

Dead and misleading code

• _elog['conflict'], _elog['conflict_tags'] and skip_filtered are never written (engine.py:2535, read at 2722 and 2734). The diagnostic prints "0 boundary-conflicts", which reads as "checked, found none" when nothing is checked.
• self.diagnostics is only reset in __init__, never in generate() — which does reset triangular_quality and element_grid. A second run can carry stale embedding data.
• plan_line_strip:947 and plan_polygon_band:1004 hard-code simplify(lc * 1.5) on the user's geometry before offsetting. Non-overridable and unwarned. Having a hidden second simplification alongside the new explicit simplify_tolerance is confusing — for a meandering river at coarse lc it can move the strip noticeably off the feature.
• utils.py now has two different ortho_error conventions: build_connectivity computes deviation from 90° (connector vs shared edge), calculate_orthogonality computes deviation from 0° (connector vs edge normal). Same number, opposite docstrings, same column name.
• engine.py:604 logs text beginning "WARNING: heal_tolerance..." at INFO level. Roughly ten other sites use logger.warning(f"Warning: ..."), where the prefix duplicates the level — leftovers from the print() conversion.
• launch_gmsh_gui opens the pre-fragment window only when verbosity > 1 (line 1544) but the post-mesh window unconditionally (line 2947). Undocumented asymmetry.

---

Tests

Coverage is much better overall — two gaps stand out:

• No idempotency test for ConceptualMesh.generate(). That's how the destructive-state bug from my first comment survived.
• _apply_transfinite_strip can apply curve constraints to one side, then bail when _distribute_chain_points returns None for the other (line 2027), returning False with the surface already partly constrained. The caller then tries the next corner set against that dirty surface. _set_default_buffer_curve_divisions overwrites most of it, so it largely self-heals, but the ordering is fragile and untested.

---

Structure

engine.py is 2,955 lines, and _add_geometry spans 725–1778 — roughly 1,050 lines holding about 20 nested closures. Several hundred more lines are if self.verbosity >= 2: diagnostic blocks interleaved with production logic (1548-1632, 1705-1727, 2507-2529, 2718-2746, plus both remap methods).

I didn't find a defect inside the structured-buffer machinery, but that's partly the point — at this size it's very hard to be confident either way, and it's where the subtlest geometry lives. Two extractions would help a lot:

1. The structured-buffer code (planning, trimming, crossing arbitration, transfinite application) into its own module, the way fields.py was split out.
2. The DIAG blocks into _diagnostics.py helpers behind single calls — or dropped now that the features work.

fields.py itself reads well. Two small inconsistencies: ThresholdField.__init__ does bare float() casts while ExponentialField and GeometricGrowthField validate through _positive_finite (it's public and in __all__, so it should too), and self.size_max if self.size_max else background_lc at line 263 treats size_max=0.0 as unset — is not None is safer.