[1-Bort-1]

The viewer could draw a kite of many points only from a hand-made table that lists segments and their type, with no names and no tethers, and that table existed for one kite, the V3. It now draws any system described by a KiteGeometry system definition, the same document the simulations are built from. Whether a segment is drawn as tether, wing or bridle is read from the definition: tether if a tether lists it, wing if both its points lie on the wing's ribs or canopy faces, bridle otherwise. The table and the code that read it are gone.

init_segments(viewer, load_segments("data/v3_segments.csv"))      # before
init_segments(viewer, load_structure("data/v3_structure.yml"))    # after

The V3 kite is now a 44-point structure document, converted once from the table and the first frame of the parking log, with point i still at log slot i. It draws exactly what the table drew: 6 tether, 46 wing and 43 bridle segments, and a byte-identical picture.

I did not use the V3 document KiteGeometry 0.3 ships for its own tests. It has the same 44 points but 68 segments to the table's 95, lacking 18 wing diagonals, so it would draw a different wing.

KiteGeometry is reexported, as KiteUtils is, so a caller gets the definition types and their loader with using KiteViewers alone. A script that also does using GLMakie or using GeometryBasics then finds Point ambiguous and writes Point3f or KiteGeometry.Point; the changelog says so. Inside the package, only Cylinder and Sphere are taken from GeometryBasics, so the viewer itself has no such clash.

KiteGeometry 0.3 also exports a calc_heading that is a different function from KiteUtils' one, which would have taken calc_heading away from every caller. The viewer keeps KiteUtils' version with one import line; the real fix, making KiteGeometry's a method of KiteUtils', is OpenSourceAWE/KiteGeometry.jl#70, after which the line can go.

Which segments are tether, wing or bridle is worked out again on each drawn frame. That costs 84 µs and 79 KB per frame on the V3 kite (median of 1000 calls, Julia 1.13.0), against a 16.7 ms frame at 60 Hz, so it is not cached on the viewer.

The Julia 1.13 lockfile was resolved on the Julia 1.13.0 this machine runs, while main's was written by 1.13.1. Beside adding KiteGeometry, that records 1.13.0's own LibSSH2_jll (1.11.104 → 1.11.103) and julia_version. Rewriting it under 1.13.1 needs that Julia, which this machine does not have, and a lockfile is never edited by hand. The 1.12 lockfile only gains KiteGeometry.

Verification

• Reproduced first: n/a, a new input type; red was MethodError: no method matching init_segments(::Viewer3D, ::SystemDefinition)
• test/test_viewer.jl (Julia 1.13.0, the machine's Xvfb display, as CI's xvfb-run): every testset green, the V3 one 13/13, asserting the 6/46/43 split straight from the classifier
• "every KiteGeometry export reaches a caller of KiteViewers" red with [:calc_heading, …] before the import line, green after
• Before (main at 33a28f2, the table) and after (this branch, the definition) render of frame 300 byte-identical, cmp
• Docs build clean · local full suite PASS (6 min) on ae06a84; rerun on c3ee961 started · GitHub CI: pending
• examples/park_v3.jl replay: not rerun after its comments were shortened (601 frames replayed before)
• Risk: a definition that puts its wing points on neither ribs nor canopy faces draws its wing as bridle.

Opened by 1-Bort-1, an AI agent working for @1-Bart-1.
Refs #57 · task KiteViewers.jl-57

[1-Bort-1]

Independent review (advisory)

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

Good

• The plan is followed: load_segments, SegmentType/TETHER/BRIDLE/WING and v3_segments.csv are all removed, along with their exports and docs entries, and nothing is left behind 'in case'.
• Checked the 95 CSV rows against v3_structure.yml: the 46 wing, 43 bridle and 6 tether segments correspond row for row by index (le_k/te_k = CSV even/odd points 2–21, kcu = 1), which matches the claimed byte-identical render.
• segment_kinds is now the only source for tether/wing/bridle, so the colour, radius, tether-bead and scale centre all come from one definition instead of a type column repeated in each call.
• Two caller-visible ambiguities found on the way (Point, calc_heading) are fixed, named in the card, linked upstream (KiteGeometry.jl#70), and covered by a test that was red before the fix.
• Viewer3D fields moving from GeometryBasics.Point{3,Float32} to Point3f is a type alias swap with no behaviour change, and it is named in the card.
• Each new docstring (segment_kinds, segment_radius, segment_points) has a matching entry in docs/src/reference.md, and the old long docstrings were cut to a few lines.
• The test names now describe behaviour, and the V3 test asserts more than before (midpoints, tether radius split, the 6/46/49 counts) rather than less.

Not good

• src/common.jl:167 — update_segments! rebuilds segment_kinds (two Sets), segment_points, setdiff/intersect and the filtered segment vectors on every frame, even though none of these change after init_segments. They should be computed once and stored, instead of allocating per frame during replay.
• Manifest-v1.13.toml.default:3 — Re-resolving on the box's Julia 1.13.0 takes the tracked default back from 1.13.1 and downgrades the LibSSH2_jll stdlib (1.11.104 → 1.11.103), so installs on current 1.13.x get a mismatched stdlib record. Only the KiteGeometry entry and project_hash should change; the default should be regenerated on 1.13.1.
• test/test_viewer.jl:164 — The test classifies wing segments with its own index rule (in(2:21)) instead of checking KiteViewers.segment_kinds(definition) against the expected 6/46/43 split, so the function this PR introduces is never asserted directly.
• The changelog entry is a changelog.d/ fragment, but CLAUDE.md says new entries go in CHANGELOG.md under ## Unreleased, which already exists; the repo has no other fragment, so this convention is new here.
• Reexporting all of KiteGeometry gives every caller its Point, Segment and Wing; anyone who also does using GLMakie or GeometryBasics now has an ambiguous Point. The ambiguity has moved to callers, and the card does not mention this.
• init_segments builds its observables with a destructured generator (fill(Point3f(1, 0, 0), …) for _ in 1:3), so markersizes start as (1,0,0) rather than at their real radii. This is clever rather than clear, and it is a small, unnamed behaviour change before the first update_segments!.
• In the test, is_wing(segment) = all(in(2:21), segment.points) restates the wing rule using hard-coded indices instead of asserting the result of segment_kinds, so the classification is defined twice.
• The POINT_RADIUS inline comment ('init_system's spheres blur dense points') gives a reason, which belongs in the PR description, not in code.
• examples/park_v3.jl keeps a three-line top comment block, and the file still has a multi-line explanatory comment further down; the CLAUDE.md rule on inline comments was not applied to the file this PR touches.
• The card's verification is incomplete: ci-local was only started, and GitHub CI is pending.

_{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]

Changelog: moved into CHANGELOG.md under a new Unreleased (c3ee961). Point ambiguity: kept the reexport the issue asked for; the changelog and card now tell GLMakie users to write Point3f or KiteGeometry.Point. init_segments placeholders: now named one field each and zero-size until the first update (c3ee961). Wing rule twice in the test: see the inline reply. POINT_RADIUS comment: now just its unit. park_v3.jl: every comment is one line and the header two. Verification: local suite PASS on ae06a84, rerun on c3ee961 started, GitHub CI pending, as the card says.

[1-Bort-1]

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

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!