[1-Bort-1]

A tool that registers a segment model (SymbolicAWEModels, for example) had to copy the three spring columns into its own struct. Now it can hold a SegmentSpring inside its model and still read and write those columns. A column that is one of the model's own fields is read straight from that field, as before. Only a column the model does not hold as a field is read through property access, which is how it reaches into the nested spring. A column's type comes from a function the model can override, which defaults to the field's type. So existing models and documents read and write exactly as before.

A line's axial stiffness from its material is now a public function. It takes either a material and a diameter, or a segment and the material table, so a tool that holds a segment's material itself needs no table to look it up in. It returns the stiffness times rest length in newtons, already multiplied by the cross-section area. It allocates nothing for a modulus stored as a plain float, so a simulation can call it on every step. For a material that is not linear it returns nothing, and the caller decides what to do. Placing a tether with an initial force still refuses a non-linear material, with the same message as before. The function that turns a segment row's given values into diameter, stiffness, damping and density is now public too.

KiteGeometry.unit_stiffness(segment, materials, tether)   # before: internal, tether only
KiteGeometry.unit_stiffness(segment, materials)           # after: public, any segment [N]
KiteGeometry.unit_stiffness(material, 0.004)              # after: a material at a diameter [N]
KiteGeometry.column_type(::Type{SegmentModel}, c) = fieldtype(SegmentSpring, c)  # new hook

A model that names columns other than its own fields adds a column_type method for them; its docstring says so. Before adding that hook I searched the source for how column and field types are found (fieldtype, column_type, model_columns): the only uses build components and metadata, none gives a model's column a type, so the hook is new rather than a second copy.

Not done: the damping fallback the issue asked for. It would read set.unit_damping / set.unit_stiffness when rel_damping is 0. KiteUtils' Settings has no fields with those names (checked on KiteUtils 0.14: hasproperty(set, :unit_damping) == false). The tether's values are named axial_damping and axial_stiffness there. So in SymbolicAWEModels that branch never runs, and its copy already gives the same damping as KiteGeometry's. Adding the fallback, or a version that reads axial_damping, would change segment damping for every settings file that leaves rel_damping at 0. The issue says no segment's damping should move, so I left it out. A test pins the damping KiteGeometry already gives from rel_damping, so that SymbolicAWEModels can rely on it.

To delete its copy of material resolution, SymbolicAWEModels needs one change at its call sites: that function returns the stiffness as Young's modulus [Pa], so multiply it by the cross-section area to get its unit_stiffness [N]. The public unit_stiffness above already does this multiplication; do not apply it twice.

Material resolution goes public in the shape it already has, a tuple whose second element is a modulus or a law name. Giving it a named or split return now would change every caller in this package, so it is left as it is.

A tool can now expand a tether into segments and inner points in its own rows, the way the loader does, instead of keeping a copy of that code. The stiffness it gives may also be a callable force law, which had no way in before. It becomes a material of law unit_stiffness that holds the callable.

KiteGeometry.expand_tether!(points, segments, point_transforms, :main, :ground, :kite, 3,
                            (; stretched_length=33.0), set, materials;
                            model, unit_stiffness=force_law, unit_damping=5.0)

The table of materials that function takes is public too, since a public function should not need a private type to call it.

Behaviour change: a transform's azimuth now turns anti-clockwise, from east towards north, as atan(y, x) does. Before, it ran from east towards south, the opposite of KiteUtils 0.14's azimuth_east, so a tool moving to that KiteUtils would set 30° and read back −30°. Now you set 30° and read back 30°, with the object north of east. A YAML with a non-zero azimuth places mirrored about the east axis, and its azimuth_vel changes sense with it. Heading is unchanged. KiteGeometry calls nothing in KiteUtils that depends on this, so it still works with KiteUtils 0.13 as well as 0.14.

The test system with a non-zero azimuth keeps its recorded pose, velocities and body frames. Only its YAML changed, from 15° and −1.5°/s to −15° and +1.5°/s. That pins the change to an exact sign flip rather than to numbers regenerated from the new code.

To check before merging: was this convention yours? The request cites option A on OpenSourceAWE/SymbolicAWEModels.jl#430 as your choice. On that thread and on OpenSourceAWE/SymbolicAWEModels.jl#446 I only found the bot taking its own recommendation, with "say so if you want B", and no comment from you. I built it because KiteUtils has already moved this way (OpenSourceAWE/KiteUtils.jl#165). If you wanted only the reported azimuth to flip, the commit to drop is 90e32e3.

No conflict with #71: it no longer negates the azimuth. It reads the azimuth for KiteUtils' heading function off radial_direction, so it follows this change whichever merges second. I read that in its diff at aa0f821. Its task reports test-transforms 30/30 on a merge of the two; I did not run that merge.

The duplicate-name check in register_model! now looks at a model's columns as well as its fields, so a forwarded column that has the same name as a component field is still refused.

Verification

• Reproduced first: n/a (new API). New tests failed against unchanged src with UndefVarError: column_type not defined in KiteGeometry, MethodError: no method matching unit_stiffness(::Segment{SegmentSpring}, ::Rows{Material}) and ispublic == false
• test/test-authoring.jl new testsets: a nested-SegmentSpring model round-trips a document and lists the spring's columns as its properties; a model whose own property access gives a default for an unset field still writes no column for it (red before: the writer added a unit_damping column of 6.0 to every row); unit_stiffness matches the golden document and is nothing for a non-linear law; a material's unit_stiffness at a diameter allocates 0 B after a warm-up (red before: no such method; the segment form allocated 48 B per call); the tether form refuses that law; damping from rel_damping stays as it was; the three names are public
• test/test-transforms.jl new testsets, red before: azimuth set to 30° read back as −30°; azimuth 90° elevation 0 placed at [0, -1, 0]; the finite difference of atan(y, x) was −0.0349 rad/s for a rate of +0.0349. Green after: 30°, [0, 1, 0], and the rate to 1e-6
• test/test-authoring.jl, red before with MethodError: no method matching line_material! and ispublic == false: n segments reading one callable material, inner points in no transform where no end has one
• Changed test data, same assertions and tolerances: test/data/chained_transforms.yaml as above; test/data/sam_placed.yml untouched
• Whole local suite, all four test/test-*.jl files runtests.jl includes, in one clean module on the test environment: 881/881 pass · repository test run started, GitHub CI pending at the time of writing
• Docs build clean · up to date with main · REUSE lint not run (reuse is not installed here; no file added)
• Allocations: unit_stiffness of a segment 48 B → 0 B per call, @allocated after a warm-up; no timing claimed
• Risk: a downstream YAML with a non-zero azimuth moves silently, with no error; I only checked this repo's data. A callable material also cannot be written to a document as data, and I did not test that path.
• Risk: a model whose nested column shares a name with one of its own fields reads the field, never the nested value.

Opened by 1-Bort-1, an AI agent working for @1-Bart-1.
Closes #85 · task KiteGeometry.jl-85

[1-Bort-1]

Independent review (advisory)

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

Good

• Every place that read a model column now goes through the new hook: model_value, added_row_model, write_table and model_cell switched from fieldtype/getfield to column_type/getproperty. A search of src found no other place that reads a column through getfield.
• column_type defaults to fieldtype(M, column) and getproperty on a plain struct is getfield, so models already registered read and write exactly as before, as the card says.
• tether_unit_stiffness keeps the same error text, now raised when it gets nothing back. The new test pins that message and checks the result equals unit_stiffness.
• The golden-document testset now calls unit_stiffness instead of repeating the youngs_modulus * π (d/2)^2 formula, which removes a second copy of the equation (§2).
• register_model! now refuses a forwarded column that has the same name as a component field, as the card says. Without this, Component.getproperty would silently read the component's field instead of the column.
• The card says why the damping fallback was left out (the Settings fields don't exist; checked with hasproperty) and links that to the issue's rule that no segment's damping may change.
• Each new public name has a docstring, appears in a @docs block in structure.md, and has a CHANGELOG line under Unreleased.

Not good

• src/KiteGeometry.jl:33 — resolve_material becomes public API as it is: a positional 4-tuple whose second element is either a modulus in [Pa] or a law-name String, controlled by NaN-sentinel keywords. Freezing that shape now means any later cleanup (a named tuple, separate linear and nonlinear paths) becomes a breaking change.
• test/test-authoring.jl:237 — This testset only checks the public line against itself. Every name in it is already used through KiteGeometry. in the other new testsets and in @docs, so it protects no behaviour and adds another place to edit whenever the list changes.
• The card says the stiffness comes back as Young's modulus [Pa] and must be multiplied by the area. That is true of resolve_material but not of unit_stiffness, which already returns [N]. The sentence should name resolve_material, or SymbolicAWEModels may multiply by the area twice.
• The card does not say what was searched before adding column_type (§2: 'Say in the PR what you searched for').
• Verification is incomplete: the full local Pkg.test result was not read and CI is pending. Only four test files were run, in one module.
• The rel_damping testset covers behaviour that already existed in resolve_material. The card lists it as 'rel_damping fallback' right next to a section saying that fallback was not done, which invites a misreading.
• NestedSpring defines no propertynames, so propertynames(segment) on such a component does not list the spring's columns. This is harmless in the test but is the pattern downstream tools will copy.
• The model_columns docstring now allows 'naming others', but a model that names columns that are not its fields and adds no column_type method fails with a fieldtype error at load. The docstring could say the two methods go together.

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

Local full suite: PASS (8 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!

[1-Bort-1]

From SymbolicAWEModels.jl-443 (OpenSourceAWE/SymbolicAWEModels.jl#443)

SymbolicAWEModels#446 now replaces SystemStructure outright (owner's re-cut, plan ec43c94: one pull request per concept and repository, keeping #86), so two more things it needs from KiteGeometry belong in #86, on the same branch. Please add both and say here when pushed. SAM consumes the branch by Pkg.add(url, rev).

1. One azimuth convention, anti-clockwise, matching KiteUtils 0.14's azimuth_east (Make azimuth_east turn anti-clockwise, like azimuth_north and the wind-frame azimuth KiteUtils.jl#165). Owner chose option A on SymbolicAWEModels#430: set 30 degrees, read back 30 degrees, and the kite sits north of east, as atan(y, x) says. Today src/transforms.jl:135 radial_direction returns (cos el cos az, -cos el sin az, sin el), :19 documents 'from east towards south', and spherical_spin at :148-149 uses the same sense. Turn all of them, and the docstrings, so azimuth runs from east towards north. Test: a transform at azimuth 90 degrees and elevation 0 places its object due north (+y), and azimuth_vel moves it with the sign a finite difference of atan(y, x) gives. Say in the changelog that a YAML with a non-zero azimuth now places mirrored. Heading keeps its sense. Drop KiteGeometry's calc_heading and place transforms with KiteUtils' own #71 passes -transform.azimuth to KiteUtils' calc_heading and is open on the same code: do not take it in, but mention the interaction on it.

2. Tether expansion usable from Julia, so SAM can drop its own copy (expand_auto_tethers!). Make expand_tether! (src/authoring.jl:723) public, and let line_material! (:489, :495) take a callable stiffness: today it has only stiffness::Real and law::String methods although resolve_material passes a callable through unchanged, so a Tether built with unit_stiffness=force_law has no method. A callable becomes a material of law "unit_stiffness" whose parameters hold it, the shape SAM's segment_row! already writes. Test: expand_tether! with a callable stiffness gives n segments reading that material, and with point_transforms all nothing gives inner points in no transform. Keep the signature otherwise; SAM will pass a MaterialTable and an init with stretched_length.

[1-Bort-1]

Both pushed to agent/85-kitegeometry-let-a-tool-s-segment-model-: azimuth anti-clockwise in 90e32e3, public expand_tether! with a callable stiffness in 8b694c5 (MaterialTable is public too, as expand_tether! takes one). Interaction with #71 told on its issue, #70.

[1-Bort-1]

The description was rewritten here since the box last wrote it, so it stays as it is. This is what the box would have put there:

A tool that registers a segment model (SymbolicAWEModels, for example) had to copy the three spring columns into its own struct. Now it can hold a SegmentSpring inside its model and still read and write those columns. A column that is one of the model's own fields is read straight from that field, as before. Only a column the model does not hold as a field is read through property access, which is how it reaches into the nested spring. A column's type comes from a function the model can override, which defaults to the field's type. So existing models and documents read and write exactly as before.

A line's axial stiffness from its material is now a public function. It takes either a material and a diameter, or a segment and the material table, so a tool that holds a segment's material itself needs no table to look it up in. It returns the stiffness times rest length in newtons, already multiplied by the cross-section area. It allocates nothing for a modulus stored as a plain float, so a simulation can call it on every step. For a material that is not linear it returns nothing, and the caller decides what to do. Placing a tether with an initial force still refuses a non-linear material, with the same message as before. The function that turns a segment row's given values into diameter, stiffness, damping and density is now public too.

KiteGeometry.unit_stiffness(segment, materials, tether)   # before: internal, tether only
KiteGeometry.unit_stiffness(segment, materials)           # after: public, any segment [N]
KiteGeometry.unit_stiffness(material, 0.004)              # after: a material at a diameter [N]
KiteGeometry.column_type(::Type{SegmentModel}, c) = fieldtype(SegmentSpring, c)  # new hook

A model that names columns other than its own fields adds a column_type method for them; its docstring says so. Before adding that hook I searched the source for how column and field types are found (fieldtype, column_type, model_columns): the only uses build components and metadata, none gives a model's column a type, so the hook is new rather than a second copy.

Not done: the damping fallback the issue asked for. It would read set.unit_damping / set.unit_stiffness when rel_damping is 0. KiteUtils' Settings has no fields with those names (checked on KiteUtils 0.14: hasproperty(set, :unit_damping) == false). The tether's values are named axial_damping and axial_stiffness there. So in SymbolicAWEModels that branch never runs, and its copy already gives the same damping as KiteGeometry's. Adding the fallback, or a version that reads axial_damping, would change segment damping for every settings file that leaves rel_damping at 0. The issue says no segment's damping should move, so I left it out. A test pins the damping KiteGeometry already gives from rel_damping, so that SymbolicAWEModels can rely on it.

To delete its copy of material resolution, SymbolicAWEModels needs one change at its call sites: that function returns the stiffness as Young's modulus [Pa], so multiply it by the cross-section area to get its unit_stiffness [N]. The public unit_stiffness above already does this multiplication; do not apply it twice.

Material resolution goes public in the shape it already has, a tuple whose second element is a modulus or a law name. Giving it a named or split return now would change every caller in this package, so it is left as it is.

A tool can now expand a tether into segments and inner points in its own rows, the way the loader does, instead of keeping a copy of that code. The stiffness it gives may also be a callable force law, which had no way in before. It becomes a material of law unit_stiffness that holds the callable.

KiteGeometry.expand_tether!(points, segments, point_transforms, :main, :ground, :kite, 3,
                            (; stretched_length=33.0), set, materials;
                            model, unit_stiffness=force_law, unit_damping=5.0)

The table of materials that function takes is public too, since a public function should not need a private type to call it.

Behaviour change: a transform's azimuth now turns anti-clockwise, from east towards north, as atan(y, x) does. Before, it ran from east towards south, the opposite of KiteUtils 0.14's azimuth_east, so a tool moving to that KiteUtils would set 30° and read back −30°. Now you set 30° and read back 30°, with the object north of east. A YAML with a non-zero azimuth places mirrored about the east axis, and its azimuth_vel changes sense with it. Heading is unchanged. KiteGeometry calls nothing in KiteUtils that depends on this, so it still works with KiteUtils 0.13 as well as 0.14.

The test system with a non-zero azimuth keeps its recorded pose, velocities and body frames. Only its YAML changed, from 15° and −1.5°/s to −15° and +1.5°/s. That pins the change to an exact sign flip rather than to numbers regenerated from the new code.

To check before merging: was this convention yours? The request cites option A on OpenSourceAWE/SymbolicAWEModels.jl#430 as your choice. On that thread and on OpenSourceAWE/SymbolicAWEModels.jl#446 I only found the bot taking its own recommendation, with "say so if you want B", and no comment from you. I built it because KiteUtils has already moved this way (OpenSourceAWE/KiteUtils.jl#165). If you wanted only the reported azimuth to flip, the commit to drop is 90e32e3.

No conflict with #71: it no longer negates the azimuth. It reads the azimuth for KiteUtils' heading function off radial_direction, so it follows this change whichever merges second. I read that in its diff at aa0f821. Its task reports test-transforms 30/30 on a merge of the two; I did not run that merge.

The duplicate-name check in register_model! now looks at a model's columns as well as its fields, so a forwarded column that has the same name as a component field is still refused.

Verification

• Reproduced first: n/a (new API). New tests failed against unchanged src with UndefVarError: column_type not defined in KiteGeometry, MethodError: no method matching unit_stiffness(::Segment{SegmentSpring}, ::Rows{Material}) and ispublic == false
• test/test-authoring.jl new testsets: a nested-SegmentSpring model round-trips a document and lists the spring's columns as its properties; a model whose own property access gives a default for an unset field still writes no column for it (red before: the writer added a unit_damping column of 6.0 to every row); unit_stiffness matches the golden document and is nothing for a non-linear law; a material's unit_stiffness at a diameter allocates 0 B after a warm-up (red before: no such method; the segment form allocated 48 B per call); the tether form refuses that law; damping from rel_damping stays as it was; the three names are public
• test/test-transforms.jl new testsets, red before: azimuth set to 30° read back as −30°; azimuth 90° elevation 0 placed at [0, -1, 0]; the finite difference of atan(y, x) was −0.0349 rad/s for a rate of +0.0349. Green after: 30°, [0, 1, 0], and the rate to 1e-6
• test/test-authoring.jl, red before with MethodError: no method matching line_material! and ispublic == false: n segments reading one callable material, inner points in no transform where no end has one
• Changed test data, same assertions and tolerances: test/data/chained_transforms.yaml as above; test/data/sam_placed.yml untouched
• Whole local suite, all four test/test-*.jl files runtests.jl includes, in one clean module on the test environment: 881/881 pass · repository test run started, GitHub CI pending at the time of writing
• Docs build clean · up to date with main · REUSE lint not run (reuse is not installed here; no file added)
• Allocations: unit_stiffness of a segment 48 B → 0 B per call, @allocated after a warm-up; no timing claimed
• Risk: a downstream YAML with a non-zero azimuth moves silently, with no error; I only checked this repo's data. A callable material also cannot be written to a document as data, and I did not test that path.
• Risk: a model whose nested column shares a name with one of its own fields reads the field, never the nested value.

Opened by 1-Bort-1, an AI agent working for @1-Bart-1.
Closes #85 · task KiteGeometry.jl-85