Textual notation improvement

.claude/agents/textual-notation-reviewer.md

@@ -12,6 +12,7 @@ You are a master of the SysML2.NET textual notation pipeline. Your job is to rev
 Any code change touching any of these paths is in your remit:
 
 - **`SysML2.NET.Serializer.TextualNotation/Writers/`** — every `.cs` file under `Writers/`, both hand-coded partial classes (`*.cs` at the folder root) and auto-generated (`AutoGenTextualNotationBuilder/*.cs`). This includes `TextualNotationValidationExtensions.cs`, `MembershipValidationExtensions.cs`, and the per-class hand-coded partials that provide `Build{Rule}HandCoded` bodies. The namespace for all of these is `SysML2.NET.Serializer.TextualNotation.Writers`.
+- **`SysML2.NET.Serializer.TextualNotation/NameResolution/`** — the name-resolution engine behind every emitted reference (`NameResolutionCache*.cs`, `NamespaceBindingIndex*.cs`, `LocalScopeResolver.cs`, `ImportExpansion.cs`, `SegmentNaming.cs`, `ContainmentPaths.cs`, `ResolutionGraph.cs`). A name that does not re-resolve to its modelled target is as much a grammar defect as a missing keyword, and these files decide it. Ground verdicts here in KerML §8.2.3.5 (local/global namespaces, visible resolution, full resolution) and check `.team-notes/name-resolution-conformance-ledger.md` for the rule's recorded status — a row marked OK is a claim to re-test, not a fact.
 - **`SysML2.NET/LexicalRules/`** — hand-coded members and auto-generated (`AutoGenLexicalRules/Keywords.cs`, `SymbolicKeywordKind.cs`, `SymbolicKeywordKindExtensions.cs`).
 - **`SysML2.NET.CodeGenerator/HandleBarHelpers/RulesHelper.cs`** — the central code-gen logic.
 - **`SysML2.NET.CodeGenerator/Templates/Uml/*.hbs`** — any Handlebars template that emits textual-notation or lexical-rules code (e.g. `core-textual-notation-builder-template.hbs`, `core-textual-notation-shared-builder-template.hbs`, `core-lexical-*.hbs`).
@@ -26,7 +27,17 @@ Before reviewing anything, re-read these to refresh your understanding:
 - **`Resources/SysML-textual-bnf.kebnf`** and **`Resources/KerML-textual-bnf.kebnf`** — the grammar source of truth. When a rule appears in both, SysML overrides KerML.
 - The `<para>{…}</para>` XML doc on the generated public `Build{Rule}` method — the authoritative grammar fragment for that specific method. For a hand-coded partial (`Build{Rule}HandCoded`), the grammar context is the same rule — the generated sibling delegates to the hand-coded method because the generator can't produce the full body automatically.
 
-### Hypha grounding (if installed)
+### Hypha grounding — REQUIRED for any spec-dependent verdict
+
+**Whenever a verdict turns on what the specification requires, or on metamodel structure, ground it
+with Hypha and cite the clause. Do not issue such a verdict from memory, from a sibling rule, or from
+the shape of the surrounding code.** A confident-but-ungrounded reading is the failure mode this
+review exists to catch — it has produced real defects here, in both directions: a rule "confirmed"
+against a plausible prior that the clause contradicts, and a correct implementation reported as a
+defect because the reviewer inferred the rule rather than reading it.
+
+State the clause you relied on in the finding. If a verdict rests on a defined term (`parameter`,
+`namingFeature`, root `Namespace`, `redefinedFeature`), quote the definition rather than assuming it.
 
 When the Hypha plugin's skills are available in your session, use them instead of reasoning from memory:
 

CLAUDE.md

@@ -6,6 +6,61 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 
 SysML2.NET is a .NET C# SDK implementing the OMG SysML v2 specification (based on Beta 4 pilot implementation). It provides metaclass DTOs/POCOs, serializers (JSON, XMI, MessagePack), a REST client, a DAL layer, and a Blazor WebAssembly viewer application. Current version: 0.19.0.
 
+## Project goal: conformance to the SPECIFICATIONS
+
+**The target for EVERY part of this SDK is conformance to the OMG specifications — not parity with
+any reference implementation.** This is the standing intent across the whole solution, not a
+textual-notation concern. Treat it as governing whenever the two conflict.
+
+The specifications in scope, and what each governs here:
+
+| Specification | Governs |
+|---|---|
+| **KerML 1.0** | abstract syntax (metaclasses, properties, multiplicities, ordering, redefinitions), derived properties and operations (OCL), invariants, model-level semantics including implied relationships (§8.4.2) |
+| **SysML v2.0** | the systems layer on top of KerML — same three concerns — plus the textual concrete syntax |
+| **Systems Modeling API & Services 1.0** | the PIM types and services, and their REST/HTTP binding (`SysML2.NET.REST`, `PIM/`, `SysML2.NET.Serializer.Dictionary`) |
+| **Model interchange** | the project/archive interchange format (`SysML2.NET.Kpar`) |
+
+This applies per layer, with the same standard everywhere:
+
+- **Abstract syntax** (`Core/AutoGen*`) — structure comes from `Resources/*_only_xmi.uml`, the single
+  source of truth. Multiplicity, `ordered`, redefinition/subsetting are contractual, not incidental.
+- **Derived properties and operations** (`Extend/`) — the OCL in the XMI is the contract. Translate
+  it faithfully AND to its intent; where the OCL is terse or leans on a defined term, ground the
+  intent (`hypha:spec-citation`) rather than guessing from a sibling.
+- **Constraints/invariants** — a validation rule is conformant only if it implements the stated
+  invariant, not an approximation that happens to pass the corpus.
+- **Concrete syntax** — the KEBNF and the clauses it comes from; deviations recorded, never silently
+  encoded.
+- **Semantics** — implied relationships and library specializations follow the clause, not what makes
+  a particular model render nicely.
+- **API/PIM** — service behaviour and payload shape follow the API specification, not the behaviour
+  of a particular server.
+
+Rules that hold at every layer:
+
+- **The specification decides.** Where a reference implementation and the specification disagree, the
+  specification wins and the divergence is recorded (`GRAMMAR.md` "Known KEBNF divergences", the
+  deviation ledger, or `GrammarErrata.cs`). Never implement a behaviour whose only justification is
+  "the reference implementation does it".
+- **A reference implementation or its corpus is a test oracle, not the definition of correct.** The
+  pilot's `.sysml` files, a reference API server's responses, a sample model — all catch regressions
+  cheaply, and agreement is usually evidence of correctness. A diff is a question, not a verdict.
+  Note specifically that the pilot's serializer replays the author's original source text wherever
+  the abstract syntax records no choice (optional keywords, name spellings, formatting), so on those
+  points it is not an authority at all — which is what the ledger's ACCEPT categories encode.
+- **Where the spec is genuinely ambiguous**, pick a reading, cite the clause, and say in one line
+  that the clause admits more than one — do not silently encode one reading as fact.
+- **Where the spec offers several valid forms and ranks none**, the choice is ours: make it a setting
+  (see issue #359) or state it as a house convention. Do not present it as a requirement.
+
+**Not yet achieved anywhere — do not claim conformance for any layer.** Confidence today rests
+largely on end-to-end comparison against sample data, which measures non-regression rather than
+conformance. What each layer needs before conformance can be asserted: **round-trip tests** (write →
+re-read → assert the same model; this exists for no serializer today, including the textual writer)
+and **per-invariant unit tests** so an individual rule — a derivation, a constraint, a resolution
+rule, a service contract — is falsifiable in isolation rather than only via an end-to-end diff.
+
 ## Build & Test Commands
 
 ```bash
@@ -224,6 +279,57 @@ Direct pushes to `development` or `master` are forbidden. All work lives on a fe
 
 **Why this split**: the user is the reviewer of record. The commit is the review and the push is the delivery — both are the user's calls. The agent's git involvement is bounded to: (a) create the branch locally + push the empty ref (so the user's push later is frictionless), and (b) leave the rest alone. This was tightened after two failures: first the agent auto-pushed branches to `development` directly, then over-corrected by auto-committing on the user's behalf.
 
+## Comments: write as few as possible
+
+Comments break readability. Default to **none**; every comment kept needs a justification. This
+applies to production code, tests, and the generator alike.
+
+- **Delete rather than write.** Prefer a better name or an extracted method over an explanation.
+- **Never narrate the what** — a comment paraphrasing the line below it is noise.
+- **No history, no benchmarks, no "this used to…"/"previously"/"an earlier approach"/"was reverted".**
+  Source control holds it. Never put timing measurements in code.
+- **No worked examples, no specific case names.** Never justify a rule with a particular model, a
+  validation-corpus file (`13a-Model Containment`, `ISQ::mass`, `first start;`), or an issue number.
+  State the rule; if it needs authority, cite the clause (`KerML §8.2.3.5.3`) and nothing more.
+- **Never reference the OMG pilot implementation** — or any other tool — as the reason for behaviour.
+  The writer implements the **specification**, not another implementation's choices. Where the spec
+  is genuinely ambiguous, say so in one sentence with the clause; where the KEBNF is defective, that
+  belongs in `GrammarErrata.cs` with its rationale, not scattered through the writers.
+- **No notes to future editors** ("keep in step with X", "do not remove"). Encode it in a guard or a
+  test.
+- **XML docs still required on every type and member** (`DEVELOPMENT_STANDARDS.md` §5.1), but held to
+  one sentence per tag, two as the ceiling — no `<para>` elaborations, no essays.
+
+### The bright line — the ONLY test for keeping an inline comment
+
+Earlier wording said to keep "a non-obvious *why*". That is not testable and gets self-served. The
+rule is:
+
+> **A comment may ONLY state a constraint that would cause a reader to break something if they did
+> not know it. A comment may NEVER explain why the change was made.**
+
+Apply it as a question with a yes/no answer: *if a reader deleted or rewrote this code without the
+comment, would they introduce a defect?* No → delete the comment. "It helps the reviewer understand
+my change" is not a yes; that belongs in the commit message.
+
+**Budget: at most 2 added comment lines per change.** Over that, delete until it fits or ask first.
+
+**Signature words that mean you are writing a commit message, not a comment.** If an added comment
+line contains any of `now`, `previously`, `rather than`, `instead of`, `used to`, `was `, `no
+longer`, `we `, `I `, or restates a `<remarks>` already on the same member — delete it. A
+`PreToolUse` hook rejects these on `Edit`/`Write`, so it fails loudly rather than reaching review.
+
+Worked example of the failure, from this repo:
+
+```csharp
+// The supplier now records each resource's root as it is read rather than re-deriving it, so
+// interior elements no longer reach this list and no metaclass filter is needed to keep them out.
+var otherRootNamespaces = globalNamespaces?.Where(c => c != null && !ReferenceEquals(c, rootNamespace))
+```
+
+Two banned signatures (`now`, `rather than`), it annotates a self-evident `Where`, and deleting it
+costs a reader nothing. The commit message was the right home for all of it.
+
 ## Quality rules
 
 - **OCL index base is 1-based; translate positional access accordingly and NEVER mix the two forms.** OCL collections are 1-based (`->at(1)` is the first element; `->first()` ≡ `->at(1)`). Two correct C# forms, applied by target:

SysML2.NET.CodeGenerator/Extensions/GrammarErrata.cs

@@ -90,6 +90,22 @@ public static class GrammarErrata
                 "parameter path emits 'out verdict', which re-parses as a plain FeatureMembership with " +
                 "direction out and so loses the metaclass. CalculationBodyItem is already declared in the " +
                 "same file, so the replacement resolves without any further correction."),
+            new("OccurrenceUsagePrefix",
+                "OccurrenceUsagePrefix : OccurrenceUsage =\n    BasicUsagePrefix",
+                "OccurrenceUsagePrefix : OccurrenceUsage =\n    UnextendedUsagePrefix",
+                "SysML 8.2.2.9.2 builds OccurrenceUsagePrefix on 'BasicUsagePrefix', which contains no " +
+                "'EndUsagePrefix', so 'isEnd' is UNREACHABLE for every occurrence usage — 'end port p1: P;' " +
+                "and 'end item a;' cannot be written at all. This is issue #124 item 7, already recorded as " +
+                "an accepted divergence in SysML2.NET.CodeGenerator/GRAMMAR.md. Three sources say the " +
+                "notation is real: (1) the pilot's own grammar reaches the end prefix here (org.omg.sysml.xtext " +
+                "SysML.xtext, OccurrenceUsagePrefix); (2) the corpus writes it — 'end port p1: P;' in the " +
+                "Simple Tests ConjugationTest and 'end [1] item a : A' in ConnectionTest; (3) the metamodel " +
+                "sets isEnd on exactly those elements. There is no admissible alternative spelling: " +
+                "'BasicUsagePrefix' reaches 'RefPrefix', whose 'isConstant ?= constant' then renders the " +
+                "pilot's transform-set isConstant as 'constant port p1: P' — which drops isEnd AND asserts a " +
+                "constant the source never wrote. 'UnextendedUsagePrefix = EndUsagePrefix | BasicUsagePrefix' " +
+                "is declared in the same file and dispatches on isEnd, so a non-end usage keeps the exact " +
+                "BasicUsagePrefix behaviour and only an end usage changes."),
             new("DefinitionElement",
                 "    | InterfaceDefinition\n    | PortDefinition",
                 "    | InterfaceDefinition\n    | AllocationDefinition\n    | PortDefinition",

SysML2.NET.CodeGenerator/GRAMMAR.md

@@ -231,10 +231,40 @@ re-diagnosed:
 | #3 | `MetadataUsage` not wired into any dispatch point | 14-Language Extensions |
 | #9 | `SatisfyRequirementUsage` requires `assert` | 08-Requirements |
 | #10 | `CaseBodyItem` admits no `ReturnParameterMember` | 10-Analysis and Trades |
-| #11 | `EnumeratedValue` cannot carry prefix metadata (`#Security enum secret`) | 13-Model Containment, 14-Language Extensions |
+| #11 | `EnumeratedValue` cannot carry prefix metadata (`#Security enum secret`) | **CONFIRMED** — see below | 13-Model Containment, 14-Language Extensions, `Simple Tests/MetadataTest` |
 
 Items #2, #4, #5, #6 concern productions with no corpus coverage.
 
+### #124 item 11 — confirmed, and it silently DROPS a FeatureTyping
+
+```
+EnumerationUsageMember : VariantMembership = MemberPrefix ownedRelatedElement += EnumeratedValue
+EnumeratedValue : EnumerationUsage = 'enum'? Usage        ← no prefix slot
+EnumerationUsage : EnumerationUsage = UsagePrefix 'enum' Usage
+```
+
+`EnumeratedValue` has no `UsagePrefix`, so — unlike `EnumerationUsage` — nothing consumes a
+`PrefixMetadataMember` from the cursor. The consequence is worse than an unwritable keyword: the
+annotation is the FIRST entry in the value's `ownedRelationship`, so the cursor is still parked on it
+when `BuildUsage` reaches the positional `FeatureSpecializationPart` guard
+(`cursor.Current is IFeatureTyping || …`). That guard fails and **the typing is never emitted**.
+
+`Simple Tests/MetadataTest` shows it exactly — all three values carry a `FeatureTyping` to
+`ClassificationLevel`, but only the annotated one loses it:
+
+```
+enum uncl: ClassificationLevel = 0;      ← [FeatureTyping, FeatureValue]
+enum conf: ClassificationLevel = 1;      ← [FeatureTyping, FeatureValue]
+enum secret = 2 { @ Security; }          ← [OwningMembership(MetadataUsage), FeatureTyping, FeatureValue]
+```
+
+**Fix when this is taken up:** emit the annotation as a prefix and advance the cursor past it before
+delegating to `Usage`, which yields the pilot's `#Security enum secret : ClassificationLevel = 2;`.
+That is the deviation this item already licenses, and it restores the typing as a side effect.
+`BuildEnumeratedValue` is generated, so the change belongs in the generator — as a HandCoded fallback
+for this rule, the way `EntryTransitionMember` (item 8) is handled. `MetadataTest` stays out of
+validation until then.
+
 ## Model ↔ notation reconciliations (NOT divergences)
 
 Cases where the grammar offers two conformant productions for one model, so the writer must choose.
@@ -305,3 +335,40 @@ Each condition alone is refuted by one of the three; the conjunction fits all of
 
 `IsValidForDefaultReferenceUsage` still encodes the one spec-mandated case (`!IsEnd &&
 Direction.HasValue`): a directed usage is always referential, so the keyword is redundant there.
+
+### `DefaultInterfaceEnd` vs `PortUsage` — the optional `port` keyword on an interface end
+
+```
+InterfaceOccurrenceUsageElement : Usage = DefaultInterfaceEnd | StructureUsageElement | BehaviorUsageElement
+DefaultInterfaceEnd : PortUsage = isEnd ?= 'end' Usage      ← end p1: P;
+PortUsage                       = OccurrenceUsagePrefix 'port' Usage   ← end port p1: P;
+```
+
+Both alternatives of `InterfaceOccurrenceUsageElement` reach `PortUsage`, so `end p1: P;` and
+`end port p1: P;` round-trip to the same metaclass and nothing records which the author wrote.
+
+The spec settles the OPTIONALITY and one hard condition on it. SysML 2.0 §7.14.2 Interface
+Definitions and Usages (p. 109, normative): "All the end features of an interface definition or
+usage must be port usages, so the use of the `port` keyword is optional on such end features if no
+owned cross feature is declared on the end."
+
+`DefaultInterfaceEnd` has no notation for a cross feature — only `EndUsagePrefix` carries the
+`( ownedRelationship += OwnedCrossFeatureMember )?` slot — so an end that owns one MUST take the
+`port` form. `IsValidForDefaultInterfaceEnd` implements exactly that condition
+(`IsEnd && OwnedCrossFeature() == null`); where both forms are open the writer takes the
+keyword-less one, the production's first alternative. That choice is ours, not a requirement.
+
+### `BinaryConnectorPart` vs `NaryConnectorPart` — a two-end connector
+
+```
+ConnectorPart : ConnectionUsage = BinaryConnectorPart | NaryConnectorPart
+BinaryConnectorPart = ownedRelationship += ConnectorEndMember 'to' ownedRelationship += ConnectorEndMember
+NaryConnectorPart   = '(' ownedRelationship += ConnectorEndMember ',' ownedRelationship += ConnectorEndMember
+                      ( ',' ownedRelationship += ConnectorEndMember )* ')'
+```
+
+`NaryConnectorPart` admits exactly two ends, so for a two-end connector `connect a to b` and
+`connect (a, b)` are both conformant and produce the same model. Three or more ends leave only the
+n-ary form. The writer prefers the binary form at exactly two ends — a house convention, not a
+requirement. `IsValidForBinaryConnectorPart`, `IsValidForBinaryConnectorDeclaration` and
+`IsValidForBinaryInterfacePart` each count `EndFeatureMembership` children for it.

SysML2.NET.CodeGenerator/HandleBarHelpers/RuleGenerationContext.cs

@@ -125,6 +125,15 @@ public TextualNotationRule FindRule(string ruleName)
         /// </summary>
         public int LoopProgressCheckCounter { get; set; }
 
+        /// <summary>
+        /// Monotonically-incrementing counter used to produce unique role-based-consumption variable
+        /// names (e.g. <c>elementAsOwningMembership0</c>) across the emission of a single rule body.
+        /// Required because a <c>TryTake</c> out-variable shares the enclosing scope, so two role-based
+        /// consumptions of the same target type in one generated method would collide (CS0136).
+        /// Incremented by <c>RuleProcessor.TryEmitPinnedRuleConsumption</c>.
+        /// </summary>
+        public int TakenElementCounter { get; set; }
+
         /// <summary>
         /// Determines whether the next sibling element is a terminal that uses <c>AppendLine</c>
         /// (e.g., <c>{</c>, <c>}</c>, <c>;</c>), in which case a trailing space would be unnecessary.