Say why precision.json omits PreciseNumber, and what adding a storage type means [patch] - #271
Merged
Conversation
… type means [patch] precision.json lists three storage types while four alias packages ship, and nothing in the repository recorded why. "Add the storage type to precision.json" is the natural assumption when adding the next one, and it is wrong twice over: PrecisionGenerator emits StorageTypes into the core ktsu.Semantics.Quantities namespace as typeof(entry), so a package-provided type would force the core to reference that package, and the field name comes from entry.ToUpperInvariant(), which is an identifier only for a C# keyword. Records both in CLAUDE.md's source-generator section and in a new docs/physics-generator.md section, and corrects that file's generator table, which described StorageTypes as "the storage types the alias packages cover" — it is the entries of precision.json, which is not the same set. Refs #236. Does not close it: the decision the issue is really about, whether StorageTypes should exist at all, is untouched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZbSovwSDQ4i4nSzyPXmY3
This was referenced Sep 24, 2026
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.
Refs #236. Does not close it — see "What this deliberately does not do".
#236 ends on a note that holds under all three of its options:
precision.json's relationship to the alias packages should be written down, because "add the storage type toprecision.json" is the natural assumption when someone adds the fifth, and it is wrong. That part needs no decision, so it is here on its own. Same shape as ImGuiApp#434 against ImGuiApp#384.What was wrong
Four storage types ship, one per alias package —
Semantics.Quantities.{Double,Float,Decimal,Precise}.precision.jsonlists three. Nothing in the repository said why, anddocs/physics-generator.mdactively pointed the wrong way, describingStorageTypes.g.csas:That is not what it is. It is the entries of
precision.json, and the alias packages cover a fourth the file does not list. A reader who trusts that row concludes the file is the registry of supported storage types and thatPrecisewas forgotten.The two reasons, verified against the code rather than the issue
Both checked at
b0d9485:PrecisionGenerator.cs:31setsNamespace = "ktsu.Semantics.Quantities"— the core — and:53-60emitsDefaultValue = $"typeof({storageType})". Atypeof(PreciseNumber)there would makeSemantics.Quantitiesreferencektsu.PreciseNumber, undoing the separation the alias packages exist to keep.Precisecarries thatPackageReferenceprecisely so the core does not.Name = storageType.ToUpperInvariant(), so only a C# keyword yields an identifier.ktsu.PreciseNumber.PreciseNumberbecomesKTSU.PRECISENUMBER.PRECISENUMBER.Also confirmed the issue's other premise still holds: outside
Generated/, the only mentions ofStorageTypesanywhere in the repository arePrecisionGeneratorandPrecisionMetadatathemselves. There is no reader, so the omission costs nothing at runtime.Changes
CLAUDE.md— one bullet in "Working with the source generator", next to the existing alias-props bullet, stating that adding a storage type means a new alias package, its generatedbuild/*.props, and one derivedStorageConversionTests<T>class, and thatprecision.jsonis not part of it.docs/physics-generator.md— corrects thePrecisionGeneratortable row, and adds a "Storage types andprecision.json" section with the generator excerpt and both consequences spelled out.The claim about what adding a storage type does require is the repo's own:
CLAUDE.md:363already says "Adding a storage type is one derived class" ofStorageConversionTests<T>, and:47already documents the alias packages andPrecise'sPackageReference. This connects those toprecision.json, which neither did.Tests
None, and none are possible — the change is two Markdown files. Nothing is generated from either, so the
verify-generatedworkflow is unaffected by construction. Nomarkdownlintconfig or CLI exists in this repo to run against it.The factual claims were verified by reading the code named above rather than by a test, which is the available check here.
What this deliberately does not do
The decision #236 is actually about — whether
StorageTypesshould exist at all (remove it as a major change, keep it and move it out of the core, or leave it enumerating three of four) — is untouched. This change describes the current state and the trap in it; it does not pick a branch, and #236 stays open for that call. The new docs section says so and links back.🤖 Generated with Claude Code
https://claude.ai/code/session_019ZbSovwSDQ4i4nSzyPXmY3
Generated by Claude Code