[1-Bort-1]

TL;DR

march_edges no longer carries its logic in nested closures: the plane cut is cut_station, the one-direction march is march_stations, and the tip bisection inside it is tip_station. Output is bitwise identical; CLEAN_CODE.md §6 rules out logic in nested closures.

What moved

• cut(point, tangent) → cut_station(vertices, faces, point, tangent), verbatim apart from the captured mesh becoming arguments.
• march(direction) → march_stations(vertices, faces, start_le, step, direction).
• The bisection block inside march, which sat under a comment header, → tip_station(vertices, faces, prev_le, tangent, step). Not asked for in cleanup: hoist march_edges' nested cut/march closures to module-level... #377, but it was a block wanting to be a function (§3) and pulling it out is what keeps march_stations one screen. Its two inline comments became its docstring.
• march_edges now calls march_stations twice (negative_y, positive_y) rather than through a one-line forwarding closure.

The three are private; they sit next to march_edges on the Private Functions docs page.

march_edges' docstring now says only what it does itself; the plane and the stop rule are in cut_station and march_stations. The why it carried goes here instead: the cut plane is vertical so that a tip curling downward cannot tilt it toward horizontal, where the least-x leading-edge pick would jump across the wing. pt and r in the touched lines are now crossing and row.

How I checked it is the same

Evaluated the pre-change march_edges (from #376's head, 87624a0) into ObjAdapter as march_edges_old and compared its full return value with == against the new one on data/ram_air_kite/ram_air_kite_body.obj:

n_bins=30  stations=41  identical=true
n_bins=60  stations=81  identical=true
n_bins=90  stations=125 identical=true
n_bins=150 stations=211 identical=true

Verification

• Reproduced first: n/a — behaviour-preserving refactor, no failing case
• Bitwise equality against the old closure version at four step sizes (above)
• test/obj_adapter/test_obj_adapter.jl: 260/260 pass (juliaserver, Julia 1.13), and the bitwise compare above rerun, after the review round
• Local full suite (agent ci-local, Julia 1.13, one cell): PASS in 5 min on 82c629a, not rerun for the docstring and rename round · GitHub CI: runs on push
• Docs build: not run locally (docs/ has no manifest and instantiating it by hand is off-limits); CI's Documentation job builds it · REUSE: repo has no bin/reuse_lint
• Changelog: none, nothing user-visible
• Risk: none beyond a reviewer preferring tip_station stay inline

Scope

+83 / -62 across 2 files: src/obj_adapter/obj_slice.jl and three lines in docs/src/private_functions.md. The net +18 in obj_slice.jl is the three new docstrings (21 lines) plus their signatures, ends and returns, less four lines off march_edges' docstring. Stacked on #376 (march_edges takes each station's tangent from its neighbours); review after it.

Closes #377 · task VortexStepMethod.jl-377

[1-Bort-1]

Independent review (advisory)

Verdict: APPROVE WITH COMMENTS · 0 inline, 0 off the diff

Good

• All three closures are now named module-level functions as #377 asked; compared by reading the removed and added lines side by side, the only changes are that the captured vertices/faces/mid_cut.le/step became arguments and probe_mid became probe
• tip_station goes a little past #377, but the card names it and gives the reason: it was a block under a comment header (§3), and pulling it out keeps march_stations on one screen
• The two inline comments in the bisection became tip_station's docstring, and the dropped 'z dropped' comment is covered by cut_station's 'vertical spanwise plane' wording, so nothing was lost
• Bitwise == against the pre-change march_edges at four step sizes is the right evidence for a pure hoist; the card reports it with station counts
• Every line in the file is 92 characters or less (checked with a grep for 93+ characters), and all three new private functions are listed on private_functions.md, so the docs build can reach their docstrings
• Replacing the forwarding closure with two named calls (negative_y, positive_y) makes march_edges read top to bottom

Not good

• The march_edges docstring (lines 343-351, not numbered in the diff) was not compacted (§7): it still explains the vertical plane cut that cut_station now documents, and repeats the stop rule that march_stations states; it should shrink to what march_edges itself does
• That same docstring keeps a why clause ('so a tip that curls downward can't tilt the plane…'), which §4 sends to the PR; this PR was the moment to move it, since the fact now lives in cut_station
• Short names in the moved or touched code were kept: pt in cut_station's argmin/argmax, and r in the three comprehensions in march_edges; §3 asks for crossing/row, and these lines are inside the change
• rows = NamedTuple[] is an abstractly typed container that carried over unchanged; harmless for bitwise output, but a typed vector would be the from-scratch shape
• The card's 'net +24 is the three new docstrings' is only roughly true: the docstrings add about 20 lines and the signatures and ends make up the rest; it does not change the verdict

_{claude, rubric CLEAN_CODE.md. A different lab from the implementer
on purpose: a reviewer sharing its blind spots would not flag its mistakes.}

[1-Bort-1]

Fixed in 5f148b8: march_edges' docstring now says only what it does (the plane and stop rule live in cut_station and march_stations), its why moved to the card, and pt/r are now crossing/row; the card's line count is corrected. Not typing rows = NamedTuple[]: its element type follows the mesh's eltype and it carried over unchanged, so it is outside a pure hoist.

[1-Bort-1]

Local full suite: PASS (4 min, Julia 1.13.0, one cell of the matrix)

[1-Bort-1]

CI: Julia 1.12 - ubuntu-latest - x64 - pull_request failed on 5f148b8ea.

https://github.com/OpenSourceAWE/VortexStepMethod.jl/actions/runs/35670136043/job/106564760267

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!