Skip to content

Read and write a structure together with the system file it names - #88

Draft
1-Bort-1 wants to merge 7 commits into
agent/85-kitegeometry-let-a-tool-s-segment-model-from
agent/87-kitegeometry-load-and-write-a-structure-
Draft

1-Bort-1 wants to merge 7 commits into
agent/85-kitegeometry-let-a-tool-s-segment-model-from
agent/87-kitegeometry-load-and-write-a-structure-

Conversation

@1-Bort-1

@1-Bort-1 1-Bort-1 commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

A structure file used to hold its own materials and repeat what the system file says about the tether, so the two could disagree. awesIO now settles this: the system file owns the design and the structure owns the discretisation. This package now follows that. Loading a structure also reads the system file it names, and refuses it if the hash recorded in the structure does not match that file's bytes. One system can now serve several structures, which is what the V3 kite needs: its beam and particle models share one v3_system.yml.

load_structure(path)                       # before: the structure alone; now with its system, hash checked
SystemDefinition(document)                 # before
SystemDefinition(document, system_document)  # after; from_yaml, from_json and definition likewise

system_text = to_yaml(system_document([beam, psm]; name, description, note))
write(system_path, system_text)
to_yaml(structure_document(beam; system="../system_config/v3_system.yml",
                           system_sha=system_sha(system_path), description, note))

Writing a system takes the union of the definitions' materials and tethers by name, and refuses one name with two different values. YAML now comes from a small writer of our own, because the old one left a unit of - unquoted and wrote 5.5e10, which PyYAML reads as a string.

Decisions I made:

  • structure_document returns the structure only, and system_document returns the system. They are two files, and one system is written once for several structures, so a structure takes the path and hash of a system already on disk. A caller can pass a stale hash, which load_structure then refuses.
  • A system's tethers are the tethers a winch holds. V3Kite's beam authoring lists 86 bridle lines in the same table as its tether, and awesIO requires every tether of a structure to be in the system. So those rows leave both files as tethers, while their segments stay in the structure. They stay in the in-memory definition from load_authoring, so a definition read back from its files has one tether where the authored one has 87.
  • Reading refuses a tether its system does not list, or whose segments disagree with the system's material, diameter or length, with awesIO's tolerance. A log is read the same way, so a definition whose rest lengths were changed after placing no longer reads back from its log.
  • The hash is checked only when loading from disk. A parsed document has no bytes to hash, and a log carries its own system rather than pointing at a file.
  • to_yaml(definition) refuses a definition that names no system file, because that file could not be loaded. structure_document and to_json still allow it, for a log and for writing the system first.
  • A tether whose segments differ in diameter or material leaves that field out of the system rather than refusing to load, since a simulation with such a tether still has to run.
  • What a system holds beyond materials and tethers, such as kites or assembly, is kept and written back, so reading and rewriting a file loses nothing.
  • A log now also carries the system, because without it a structure's material names cannot be resolved. A log saved before this change is refused with a message saying so.
  • A callable force law is refused when writing YAML, naming the value and not the material. A log still writes it as its name, as it did, so saving a finished run cannot fail on it.
  • The V3 files hold columns SymbolicAWEModels adds, such as a point's area and a tube's damping. Reading them with this package alone is refused, naming the columns, and strict=false drops them. A silent drop would lose them on rewriting, and keeping unread columns would let a typo through. This package's tests read those files with a small stand-in for SymbolicAWEModels' models, so nothing here depends on it.
  • Vendored examples moved to test/data/structure/ and test/data/system_config/, so the relative system path they hold resolves unchanged.
  • Before adding the YAML writer I searched src/ and KiteUtils for a writer and a file hash: the only one was YAML.jl's YAML.write, which is what wrote the unquoted unit and 5.5e10.

The V3 files themselves come from this writer, through 1-Bart-1/awesIO#39, from V3Kite's own project files. Their system lists 2 tethers and 146 materials, of which 145 are linear, because each canopy spring in V3Kite has its own modulus. The beam tether's material is named bridle_line, because it has the same values as that variable, and its length is 250.00000000000006 m, summed from its segments and within awesIO's tolerance.

SymbolicAWEModels has to change these (its unit samsystem), read at its main 78601c5:

  • src/definition_loader.jl:344 passes a system to SystemDefinition, and sys_struct_from_document, save_structure_document and load_structure_document carry both files and the hash.
  • src/definition_loader.jl:326 forwards the new keywords of its structure_document method and gains a system_document one.
  • src/system_structure/definition.jl:14 gives the tethers its winches hold as system_tethers, or neither file lists a tether.
  • Anything calling to_yaml(definition) on a definition with no system file now passes through structure_document with the path and hash.
  • test/test_structure_document.jl:191,207,221 read back with the system.
  • Its [compat] bound moves to the next minor. Saving a log needs no change there, as save_log here writes both.

Stacked on #86, which merges first. Ready for review together with 1-Bart-1/awesIO#39, which merges before this. This vendors that branch at 1-Bart-1/awesIO@5dc983b, so it can't merge until it is vendored again from that merge. Each V3 file's note names f9a13c9, so this branch keeps that commit.

Breaking, so it goes out as the next minor.

Verification

  • Reproduced first: n/a, new feature
  • The whole test suite on Julia 1.13: structure 249, log 4, authoring 704, transforms 28, all pass. They include the V3 pair round trip, the refused hash, the refused union, the refused disagreeing tether, a bridle line left out of both files, a system from two authored definitions, and the writer's quoting and number forms.
  • Round trip on the real files: load_structure then the writer reproduces both structures and the system byte for byte, and a test now holds that. awesIO's own validator and its readers ran on these files in awesIO: the system file owns the design, and the examples are written by KiteGeometry 1-Bart-1/awesIO#39, not here.
  • Rewritten tests: reading tests use the fixture cut to the schema's columns; the real-file tests register the stand-in; expected values follow the files (bridle_line for v3_beam_line, the tether's length within rounding of 250 m, the wing's quaternion compared with the file's rather than with the identity, which it now misses by 3e-17). narrowed_system narrows a loaded definition, and two authoring tests compare structure_document, because to_yaml of a definition with no system file is refused. The properties they protect are unchanged.
  • Local suite on a09b620 (one cell, Julia 1.13): started, result posts on this pull request · GitHub CI: started by this push
  • Docs not rebuilt on a09b620: no public symbol changed since the last clean build, only one generated docstring. REUSE: no new files, and the lint is not installed here.
  • Benchmark: n/a
  • Risk: the stand-in models hold only the columns the V3 files use, so a column SymbolicAWEModels writes for another kite is not exercised here.

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

1-Bort-1 and others added 4 commits November 2, 2026 23:13
BREAKING. Follow awesIO b5a9a60: the system file owns materials and whole-component
tether quantities, the structure the discretisation. SystemDefinition holds both,
load_structure checks system_sha on the file's bytes, system_document writes the
union of several definitions, and to_yaml writes YAML 1.2 that 1.1 readers agree on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@1-Bort-1 1-Bort-1 added agent:running Agent task state agent:ci Agent task state agent:review Agent task state and removed agent:running Agent task state agent:ci Agent task state labels Nov 3, 2026

@1-Bort-1 1-Bort-1 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review (advisory)

Verdict: REQUEST CHANGES · 4 inline, 0 off the diff

Good

  • Hash refusal is right: load_structure hashes the named file's bytes relative to the structure and refuses a mismatch; read in src/document.jl:259-270, and test-structure.jl covers a one-byte change.
  • The union by name refuses one name with two values for both materials and tethers; read union_by_name, and tests cover both refusals.
  • The writer quotes - and writes 1.0e-12 and 5.5e+10; read src/yaml_writer.jl, and the test matches every V3 float against PyYAML's float pattern.
  • note, description, system and system_sha are caller-settable on the writers, as the issue asks; read structure_document and system_document.
  • No existing test assertion was weakened; the test diff against #86's tip only adds the system argument, moves fixture paths and adds cases.
  • The vendored v3_system.yml holds the 2 tethers and 6 materials the issue compares against, and system_document(load_structure(path)) is tested equal to it for both fixtures.
  • Renaming a material as the card suggests is safe, since Rows looks names up by scanning and references are resolved to indices; read src/rows.jl and document_value.
  • Tests and the card's V3 file checks were not rerun for this review; the above comes from reading the code and tests.

Not good

  • src/authoring.jl:1208 — Every authoring tether becomes a system tether, so the V3 system has 88 tethers and 146 materials and the beam tether's material is bridle_line, against the issue's 'that system lists both' and v3_system.yml as what the writer 'should reproduce in content' (2 tethers, 6 materials, v3_beam_line). The card hands this to the awesIO#39 review, but this unit owns it: 86 bridle lines would be published as non_conductive_tether, and the vendored beam structure's single tether no longer matches.
  • src/document.jl:135 — Nothing checks that each structure tether names a system tether, or that its segments agree with that tether's length and diameter, though the vendored schema requires the name and the card says awesIO's validator does too. A file awesIO refuses loads here (narrowed_system in the tests relies on it), which leaves open the disagreement between files that this PR exists to end.
  • CHANGELOG.md:19 — The issue says 'structure_document writes both, and the system file's hash into the structure', but it returns the structure only and the caller must write the system, hash it and pass the hash back. The card names this as its own decision, and the cost is that a caller can pair a structure with a stale or hand-typed hash, caught only at load.
  • src/document.jl:30 — An empty system or system_sha is a legal default, so to_yaml(definition) silently writes system: "" and system_sha: "", which fail the schema's minLength and pattern, and load_structure refuses that file. The new test helper schema_document injects a path to get past validation, so the exported default path is left producing invalid files with no error.
  • The vendored examples are still the script-converted ones, so the issue's re-vendor from regenerated files and from the merge is still open before this can merge.
  • No test runs system_document on the real beam and PSM pair from load_authoring; the union tests use narrowed_system copies of the beam fixture, so the issue's first use rests on the card's manual run.
  • test/test-log.jl:10 is 97 characters, over the 92-column limit of rubric §10.
  • The SystemDefinition(document, system_document) docstring explains why with 'which has the file's bytes', which rubric §4 sends to the card.
  • The card does not say what was searched for before adding the YAML writer and helpers, as rubric §2 requires.
  • yaml_flow has no method for a callable, so to_yaml of a definition with a callable-stiffness material throws a bare MethodError rather than a message naming the material.
  • definition(log) on a log saved before this change fails with a bare KeyError on system, not a message saying the log predates the system key.

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

Comment thread src/authoring.jl Outdated
system.spanwise_stations, system.pulleys, system.tethers,
system.winches, system.wings,
system.canopy_faces, bodies, system.tubes, system.extras)
system_tethers = [design_tether(system, tether, segments) for tether in system.tethers]

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR: Every authoring tether becomes a system tether, so the V3 system has 88 tethers and 146 materials and the beam tether's material is bridle_line, against the issue's 'that system lists both' and v3_system.yml as what the writer 'should reproduce in content' (2 tethers, 6 materials, v3_beam_line). The card hands this to the awesIO#39 review, but this unit owns it: 86 bridle lines would be published as non_conductive_tether, and the vendored beam structure's single tether no longer matches.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: a system now lists only the tethers a winch holds, and a structure file only the tethers its system lists, so the V3 system has 2 tethers and the 86 bridle lines stay as segments. Not fixed: the 146 materials, since each canopy spring in V3Kite has its own modulus and the 6 came from a script's stand-in, and the name bridle_line, which comes from V3Kite's variable and a caller can rename. Both are on the card.

Comment thread src/document.jl Resolved
Comment thread CHANGELOG.md Resolved
Comment thread src/document.jl Resolved
@1-Bort-1 1-Bort-1 added agent:queued Agent task state agent:running Agent task state and removed agent:review Agent task state agent:queued Agent task state labels Nov 3, 2026
@1-Bort-1

1-Bort-1 commented Nov 3, 2026 •

Copy link
Copy Markdown
Member Author

Local full suite: PASS (7 min, Julia 1.13.3, one cell of the matrix)

@1-Bort-1 1-Bort-1 added agent:queued Agent task state agent:running Agent task state and removed agent:running Agent task state agent:queued Agent task state labels Nov 3, 2026
1-Bort-1 and others added 2 commits November 3, 2026 03:20
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@1-Bort-1 1-Bort-1 added agent:ci Agent task state agent:review Agent task state and removed agent:running Agent task state agent:ci Agent task state agent:review Agent task state labels Nov 3, 2026
The regenerated examples carry SAM's columns, so the tests read them with
a stand-in for SAM's default models, and fixture_document keeps the schema's
columns for the tests about reading. The round trip now pins the writer's
bytes against all three files. Tests follow the files: bridle_line for
v3_beam_line, a beam tether length within rounding of 250 m, and the wing's
quaternion compared with the file's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@1-Bort-1 1-Bort-1 added agent:ci Agent task state agent:review Agent task state and removed agent:running Agent task state agent:ci Agent task state labels Nov 3, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:review Agent task state

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant