Semantic member metadata, and the editor controls that set it - #153
Merged
Conversation
A member could say what type it was and nothing about what it meant. This
adds the semantic layer: Unit, Range, DefaultValue, Interpolation, Network
and Editor, as direct properties on SchemaMember and on ISchemaMember.
Units come from ktsu.Semantics.Quantities rather than being modelled here.
A member stores the text -- a symbol ("m/s") or a unit name
("MeterPerSecond") -- and UnitRegistry resolves it against the 189 units in
that package, so the conversion factors and the 72 dimensional formulae
stay owned by the library that generates them instead of being copied into
every schema file. The registry is built by reflecting over the quantities
assembly rather than being a list maintained here, which would fall behind
every unit added upstream.
Symbols are not unique, and the two collisions are not harmless: 'g' is
both gram and standard gravity, and 'rad' is both radian and radiation
absorbed dose -- not even the same dimension. Resolution refuses an
ambiguous symbol and names the candidates rather than picking one; writing
the unit's name is how a schema escapes it.
Validation checks what only makes sense in combination: a unit on
something that measures nothing, a backwards range, a default outside its
range or of the wrong kind, an enum default naming a value the enum does
not have, interpolation on something with no meaningful midpoint, a
negative quantisation step. A quantisation step wider than the whole range
is a warning rather than an error -- suspicious, not invalid.
`wrap` is the case that is easy to get backwards, so it is stated
explicitly in the docs and asserted in the tests: a wrapping range is a
*period*, not a bound. Seven radians on a [0, 2pi) member is un-normalised
rather than wrong, and clamping it to the maximum is the one transformation
that is certainly incorrect. A default outside a wrapping range is
therefore accepted.
formatVersion moves to 2. The change is purely additive -- a file using
none of the new properties is byte-identical to the version 1 file it would
have been -- but a version 1 reader ignores properties it does not
recognise, so it would load such a file, drop the metadata and write it
back without it. That is exactly the information loss the round-trip
contract in docs/schema-format.md forbids, so refusing to read it is the
honest outcome.
Three defects found by looking at the emitted JSON rather than by the round
trip, which cannot see any of them:
- IsWellFormed and IsQuantised were being written to the file. A derived
property in a file is a second, independently editable copy of a fact
the file already states.
- Interpolation was written as an ordinal, so inserting a mode into the
enum would have silently changed the meaning of every existing schema.
It is written by name now, for the same reason an enum default stores a
value's name rather than its index.
- The docs said the default discriminator was `defaultKind`; it is
`DefaultKind`, matching the existing `TypeName` discriminator.
A test now asserts the emitted JSON directly, since a round trip passes
happily on all three.
Also found by running it: the name lookup was case-insensitive, so "rad"
matched the *name* Rad before the symbol check ran and silently resolved to
an absorbed radiation dose instead of reporting the ambiguity. Names are
matched ordinally now.
24 new tests. 328 pass on net10.0; net9.0 and net8.0 compile but the
runtimes are not installed here to run them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014mXjptwAwW1SgvmsQ8ej4z
The metadata a member can carry existed in the model and in the file, and nowhere a person could reach it: the editor drew a member's name, type, container and key, and none of the six properties added with it. Each member row now has a second fold beside the description one, holding a unit picker, the range and its wrap flag, the default and its kind, the interpolation mode, the network encoding, and the editor hint. Folded away rather than in the row, because the row is already five controls wide and all six are absent on most members -- an identifier measures nothing and blends with nothing. The fold's own button says whether there is anything behind it, since a folded row is the only place most members are ever seen. The unit picker is the registry, searchable by name or symbol. What it writes is UnitRegistry.PreferredText: the symbol when the symbol names one unit, and the name when it does not. A picker knows which unit was chosen, so writing "rad" for a radian -- text the resolver then refuses as ambiguous with an absorbed radiation dose -- would throw that away. A test asserts the round trip over every unit in the registry rather than a sample, since the spellings that do not survive it are the ones nobody thinks to sample. An enum member's default is offered as that enum's values rather than as free text. It is the mistake the property invites, and the schema already knows which names are allowed. Numbers go through a new EditField.Number, for the reason the rest of that class exists: ImGui's own numeric input reports its value every frame, so a bound edited through it would push an undo entry per keystroke. Text also holds what a number cannot -- "-", "0.", an empty box -- without the model seeing any of it, and unparsable text leaves the bound alone. Every edit here is one undo entry, and a range and an encoding are immutable, so each edit replaces one rather than mutating it. Two extractions came with it. The member grid moved out of SchemaEditor into MemberGridPanel and the metadata into MemberSemanticsPanel, following the CodeGeneratorPanel pattern: the editor class was one type below CA1506's coupling limit, and a panel that reaches into a member's type, its array key and now its unit belongs with the grid that edits them rather than with the editor as a whole. No probe name changed, so the existing tests moved with it untouched. Two checkboxes lost their visible labels. A checkbox's caption sits inside the item's rectangle without being clickable, so a test aiming at the middle of the control would have hit nothing. 21 new tests, driven through the real editor because an immediate-mode control's value only reaches the schema through what the widget reports. 330 pass in Schema.Test and 201 in SchemaEditor.Test on net10.0; the library also targets net9.0 and net8.0, which compile but whose runtimes are not installed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NmGpVzuXro25PVp3e1CRru
The metadata reached the file and the editor and stopped there: generated code said a member was a float and nothing about what the float meant, so a consumer reading the generated types got none of it, and reimporting them gave back a schema missing every unit, range, default, interpolation and network encoding it started with. Generated properties now carry SchemaUnit, SchemaRange, SchemaDefault, SchemaInterpolation, SchemaNetwork and SchemaEditorHint from ktsu.Schema.Runtime, and ClrTypeImporter reads them back. This is what SchemaKeyAttribute already did for a keyed map's key member, for the same reason: a CLR type cannot say it, so without the attribute the round trip loses it. The two sides are inverses, and the existing generate-compile-reimport test now compares all six, so neither can change alone. SchemaDefault is one attribute with a constructor per kind of value rather than three attributes, because a member has one default: the overload the generator picks is what says which kind it is, and the boxed value is what still remembers on the way back. The D suffix on a numeric literal is what binds that overload rather than the one taking a bool. A default is also emitted as the property's initialiser, so a generated instance starts at the default instead of merely recording what it should have been. The two are not redundant -- the initialiser makes the object right, the attribute makes the default readable off the type. A default of a kind the member cannot hold is a validation error and generation refuses an invalid schema, but the generator is public and can be called directly, so a mismatch falls through to the type's own initialiser rather than emitting source that does not compile. There is a test for that path. The reflection importer moved out of Schema into ClrTypeImporter, which is the same extraction the editor's panels needed and for the same reason: the class was one type below CA1506's coupling limit. It also reads better as a matched pair with the generator than buried in the schema's own API. Schema.AddClass(Type) is unchanged and now delegates to it. 332 pass in Schema.Test and 201 in SchemaEditor.Test on net10.0; net9.0 and net8.0 compile but their runtimes are not installed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NmGpVzuXro25PVp3e1CRru
SonarCloud's quality gate failed the pull request on reliability, for two exact floating-point comparisons the metadata validation had introduced (S1244), and the code quality bot reported the same two. Both suggested tolerances, and a tolerance would have been wrong in each case. "Is this default a whole number" is exactly answered by double.IsInteger, which asks the question directly; an epsilon would accept 1.0000000001 as whole and then lose it silently on the narrowing to int. It also answers false for an infinity, which is not a whole number and cannot narrow to one -- the comparison against Math.Truncate called an infinity whole, since truncating one returns it. The zero-width wrapping range is now checked after the well-formed check rather than beside it, which is what lets it read `Maximum <= Minimum`: the range's maximum is at or above its minimum by then, so that says the two are the same value without comparing them for equality. The well-formed check returns, because a backwards range has no width to ask about and reporting both would be two messages for one mistake -- which is the behaviour it already had, since a backwards range was never equal to itself either. Reproduced locally by building against SonarAnalyzer.CSharp before and after -- both S1244s at Schema/Models/Schema.Validation.cs, then neither, and no reliability finding in any of the files this pull request adds. Two tests for the two edges the change decides: a backwards wrapping range reports once, and an infinite default on an integral member is not a whole number. 334 pass in Schema.Test on net10.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NmGpVzuXro25PVp3e1CRru
The quality gate still failed on reliability after the last push, for a
third exact floating-point comparison that the local analyzer run had
missed: it was an incremental build, so the editor project was up to date
and never recompiled, and an analyzer that does not run reports nothing.
The clean rebuild that would have caught it is what verified this one.
Sonar's rule counts Equals on two doubles as an exact comparison, which it
is. The question the field is really asking is whether the number the user
typed differs from the one on screen, and the field is bound to text, so it
is now answered as text: render the parsed value and compare the two
strings. "1.0" and "1" still count as the same number, which is the case the
comparison existed for, and nothing compares two doubles.
Two smells in code this branch added, from the same report:
- The importer's property and field loops were the same eight lines twice,
which is what took the method past the cognitive complexity limit when
it moved to its own file. They are one ImportMember call each now; a
property and a field are read identically and differ only in where the
type comes from, so the caller supplies it.
- A generated boolean default was asserted with AreEqual(true, ...).
The remaining findings on the branch are assertion-style preferences from a
newer MSTest analyzer -- Assert.IsEmpty over Assert.AreEqual(0, ...), and so
on -- which the rest of this suite does not follow either. They are
maintainability, not reliability, and the gate does not fail on them;
changing only the new tests would leave the suite inconsistent with itself.
334 pass in Schema.Test and 201 in SchemaEditor.Test on net10.0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NmGpVzuXro25PVp3e1CRru
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Three pieces of work, plus two follow-ups from the quality gate: the model and file format for the metadata a member carries beside its type, the editor surface that lets a person set it, and the generated C# that carries it out to a consumer and back.
1. Give a member a unit, a range, a default, and how to send it
A member could say what type it was and nothing about what it meant. This adds the semantic layer:
Unit,Range,DefaultValue,Interpolation,NetworkandEditor, as direct properties onSchemaMemberand onISchemaMember. All six are optional and omitted when absent.Units come from ktsu.Semantics.Quantities
A member stores the text — a symbol (
"m/s") or a unit name ("MeterPerSecond") — andUnitRegistryresolves it against the 189 units in that package, so conversion factors and dimensional formulae stay owned by the library that generates them instead of being copied into every schema file. The registry is built by reflecting over the quantities assembly rather than being a list maintained here, which would fall behind every unit added upstream.Symbols are not unique, and the two collisions are not harmless:
gis both gram and standard gravity, andradis both radian and radiation absorbed dose — not even the same dimension. Resolution refuses an ambiguous symbol and names the candidates rather than picking one; writing the unit's name is how a schema escapes it.Name lookup is
StringComparer.Ordinal, notOrdinalIgnoreCase: with case-insensitivity"rad"matched the nameRadbefore the symbol check ran and silently resolved to an absorbed radiation dose instead of reporting the ambiguity.Validation
Checks what only makes sense in combination: a unit on something that measures nothing, a backwards range, a default outside its range or of the wrong kind, an enum default naming a value the enum does not have, interpolation on something with no meaningful midpoint, a negative quantisation step. A quantisation step wider than the whole range is a warning rather than an error — suspicious, not invalid.
wrapis the case that is easy to get backwards, so it is stated explicitly in the docs and asserted in the tests: a wrapping range is a period, not a bound. Seven radians on a[0, 2π)member is un-normalised rather than wrong, and clamping it to the maximum is the one transformation that is certainly incorrect. A default outside a wrapping range is therefore accepted.formatVersion moves to 2
Purely additive — a file using none of the new properties is byte-identical to the version 1 file it would have been — but a version 1 reader ignores properties it does not recognise, so it would load such a file, drop the metadata and write it back without it. That is exactly the information loss the round-trip contract in
docs/schema-format.mdforbids, so refusing to read it is the honest outcome.Three defects found by looking at the emitted JSON
A round trip passes happily on all three, so a test now asserts the emitted JSON directly.
IsWellFormedandIsQuantisedwere being written to the file. A derived property in a file is a second, independently editable copy of a fact the file already states. Now[JsonIgnore].Interpolationwas written as an ordinal, so inserting a mode into the enum would have silently changed the meaning of every existing schema. It is written by name now.defaultKind; it isDefaultKind, matching the existingTypeNamediscriminator.2. Let the editor set a member's unit, range, default and encoding
The metadata existed in the model and in the file, and nowhere a person could reach it: the editor drew a member's name, type, container and key, and none of the six.
Each member row now has a second fold beside the description one, holding a unit picker, the range and its wrap flag, the default and its kind, the interpolation mode, the network encoding and the editor hint. Folded away rather than in the row, because the row is already five controls wide and all six are absent on most members — an identifier measures nothing and blends with nothing. The fold's own button says whether there is anything behind it, since a folded row is the only place most members are ever seen.
UnitRegistry.PreferredText: the symbol when the symbol names one unit, the name when it does not. A picker knows which unit was chosen, so writing"rad"for a radian — text the resolver then refuses as ambiguous — would throw that away. A test asserts the round trip over every unit in the registry rather than a sample.EditField.Number, for the reason the rest of that class exists: ImGui's own numeric input reports its value every frame, so a bound edited through it would push an undo entry per keystroke. Text also holds what a number cannot (-,0., an empty box) without the model seeing any of it, and unparsable text leaves the bound alone. Every edit here is one undo entry, and a range and an encoding are immutable, so each edit replaces one rather than mutating it.3. Carry the metadata through generated C# and back
Generated code said a member was a
floatand nothing about what the float meant, so a consumer reading the generated types got none of this, and reimporting them gave back a schema missing everything it started with.Generated properties now carry
SchemaUnit,SchemaRange,SchemaDefault,SchemaInterpolation,SchemaNetworkandSchemaEditorHintfromktsu.Schema.Runtime, and the importer reads them back. This is whatSchemaKeyAttributealready did for a keyed map's key member, for the same reason: a CLR type cannot say it, so without the attribute the round trip loses it. The existing generate-compile-reimport test now compares all six, so neither side can change alone.SchemaDefaultis one attribute with a constructor per kind of value rather than three attributes, because a member has one default: the overload the generator picks says which kind it is, and the boxed value is what still remembers on the way back. A default is also emitted as the property's initialiser, so a generated instance starts at the default instead of merely recording what it should have been — the initialiser makes the object right, the attribute makes the default readable off the type. A default of a kind the member cannot hold is a validation error and generation refuses an invalid schema, but the generator is public, so a mismatch falls through to the type's own initialiser rather than emitting source that does not compile; there is a test for that path.Floating point, twice, from the quality gate
The last two commits answer SonarCloud and the code-quality bot, which found three exact floating-point comparisons in the new code. Both bots proposed tolerances; none was taken, because in each case a tolerance changes what the check means.
int; accepting1.0000000001as whole means accepting a value that then silently becomes1— the loss the check exists to catch. It reads!double.IsInteger(number.Value)now, which asks the question directly. One deliberate consequence: an infinite default on an integral member is now reported, becauseMath.Truncate(double.PositiveInfinity)returns the infinity and the old comparison called it whole.Math.Abs(min - max) <= double.Epsilonis not a tolerance —double.Epsilonis the smallest denormal — it is equality that reads as though it forgives imprecision. The check now runs after the well-formed check, which lets it readMaximum <= Minimum: the range satisfiesmin <= maxby then, so that states equality without comparing two doubles."1.0"and"1"still count as the same number.Each has a test for the edge it decides.
Three extractions, no behaviour change
SchemaEditorandSchemawere each one type below CA1506's coupling limit, so each of these was needed to add anything at all, and each is the pattern the repository already uses (CodeGeneratorPanel):MemberGridPanel— the member grid, out ofSchemaEditor.MemberSemanticsPanel— the metadata fold.ClrTypeImporter— the reflection importer, out ofSchema.Schema.AddClass(Type)is unchanged and delegates to it.No probe name changed, so the existing editor tests moved with the grid untouched.
Testing
49 new tests. In Release on net10.0: 334 pass in
Schema.Testand 201 pass inSchemaEditor.Test, none failing or skipped. The editor tests drive the real editor headlessly, because an immediate-mode control's value only reaches the schema through what the widget reports.Schemaalso targets net9.0 and net8.0 — both compile, and CI runs all of them on Ubuntu and Windows.The remaining Sonar findings on the branch are assertion-style preferences from a newer MSTest analyzer (
Assert.IsEmptyoverAssert.AreEqual(0, …), and so on). They are maintainability rather than reliability, the gate does not fail on them, and the rest of this suite does not follow that style either — converting only the new tests would leave the suite inconsistent with itself.docs/schema-format.md,README.mdandCLAUDE.mdare updated.🤖 Generated with Claude Code
https://claude.ai/code/session_01NmGpVzuXro25PVp3e1CRru