Skip to content

feat(migrate): honour the compartments a Cameo symbol suppresses as per-symbol DiagramLayout switches - #1197

Merged
HuiJun merged 12 commits into
developfrom
devin/1792127253-cameo-suppressed-compartments
Oct 16, 2026
Merged

HuiJun merged 12 commits into
developfrom
devin/1792127253-cameo-suppressed-compartments

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

What and why

A Cameo block definition diagram migrated with sysml -migrate sysml keeps each symbol's Cameo-sized box as DiagramLayout::Layout. When the modeler collapsed compartments in Cameo the box is small (a 200×33 part def 'Imaging Subsystem' bar), and the positioned-tree renderer draws every compartment row into it, clips them and writes a "not represented" notice. The modeler's choice is in the diagram stream: each symbol's SUPPRESS_*_COMPARTMENT Boolean properties, stated on the symbol or inherited from the diagram's symbol style (symbolStyles / PropertyManagerByStereotype, referenced by the symbol's symbolStyleID).

This change honours those settings: a compartment the modeler suppressed on a symbol is written as a per-symbol show… = false switch on that symbol's DiagramLayout::Layout, and the renderers omit it for that symbol (no row, no notice). A compartment not suppressed keeps the current behaviour: drawn, clipped with a notice when the box is too small.

Representation, ranked by SysML v2 spec compliance

Each candidate was probed with the pinned pilot validate-sysml-batch (2026-08) and sysml -validate:

  1. Per-symbol Boolean show… attributes on the symbol's existing DiagramLayout::Layout (showAttributes, showParts, showReferences, showPorts, showConstraints, showItems, showActions; chosen) — user-defined metadata with literal Boolean values, each switch a declared attribute the language resolves, the shape of the existing collapsed and of a rendering's show… attributes. Pilot clean, sysml clean. Needs the DiagramLayout library change.
  2. enum def Compartment + attribute hidden : Compartment[0..*] on Layout — valid (pilot clean, sysml clean); references is a keyword, so the literals cannot mirror the compartment names.
  3. attribute hidden : ScalarValues::String[0..*] on Layout — valid (pilot clean, sysml clean); compartment names are unchecked strings.
  4. A rendering-level per-kind switch (rendering noParts :> asTreeDiagram { attribute showParts : Boolean = false; }) — standard-only text, valid, but view-wide. On SEM it loses per-symbol fidelity in most views: 18 of 38 views draw both symbols that suppress the class attributes compartment and symbols that show it (operations: 18, flow properties: 11, proxy ports: 11, full ports: 11, parts: 3).
  5. The Views library and the graphical notation offer nothing per symbol: a view filter removes an element from the whole view, not a compartment from one symbol; the standard renderings carry no compartment switches.

Kind map (Cameo compartment property → Layout switch)

Cameo property v2 compartment switch
SUPPRESS_PARTS_COMPARTMENT, SUPPRESS_IBD_PARTS_COMPARTMENT part usages showParts
SUPPRESS_REFERENCES_COMPARTMENT, SUPPRESS_IBD_REFERENCES_COMPARTMENT ref usages showReferences
SUPPRESS_PROPERTIES_COMPARTMENT, SUPPRESS_VALUES_COMPARTMENT, SUPPRESS_CLASS_ATTRIBUTES, SUPPRESS_IBD_VALUES_COMPARTMENT, SUPPRESS_IBD_UMLPROPS_COMPARTMENT, SUPPRESS_IBD_INITIALVALUES_COMPARTMENT attributes showAttributes
SUPPRESS_CONSTRAINTS_COMPARTMENT, SUPPRESS_IBD_CONSTRAINTS_COMPARTMENT constraint usages showConstraints
SUPPRESS_FLOWPROPERTIES_COMPARTMENT, SUPPRESS_IBD_FLOWPROPERTIES_COMPARTMENT item features showItems
SUPPRESS_PROXYPORTS_COMPARTMENT, SUPPRESS_FULLPORTS_COMPARTMENT, SUPPRESS_CLASS_PORTS port usages showPorts
SUPPRESS_CLASS_OPERATIONS, SUPPRESS_IBD_OPERATIONS_COMPARTMENT action usages showActions

A v2 compartment gathers what several Cameo compartments list between them (a block's class attributes and its values both become attributes), so a switch is written only when every Cameo compartment the symbol states for it is suppressed: Cameo's own block style suppresses SUPPRESS_CLASS_ATTRIBUTES while the values compartment stays shown, and showAttributes stays unset. SUPPRESS_STRUCTURE, SUPPRESS_BEHAVIORS, SUPPRESS_IBD_BEHAVIOR_COMPARTMENT and SUPPRESS_SIGNAL_RECEPTIONS map to no v2 compartment and are counted under "unsupported presentation properties, dropped".

Migrator

  • sysmlv1: a symbol's SUPPRESS_* properties are read into Symbol.Compartments; the stream's symbolStyles managers are read with their parentPropertyManager chain and a symbol inherits the settings its own properties do not state, through symbolStyleID (a property with no value is Cameo's default, shown, and overrides an inherited suppression).
  • migrate: hiddenCompartments maps them to kinds; the placement writer appends show<Kind> = false to the symbol's Layout; an MTIP export that places the symbol keeps the stream's switches with its box; the migration report gains # compartments suppressed: N on M symbols (compartmentsSuppressed, symbolsSuppressing in the sidecar).

Renderer

  • semantics.Layout reads the seven switches into Layout.Hidden (HiddenCompartments); view.Geometry carries it.
  • The positioned-tree fold (foldCompartments) leaves a hidden compartment's rows out of Node.Compartments and counts them apart in the folding notice (9 node(s) without a position: 7 drawn as compartment rows of the node above them, 2 in compartments its Layout hides left undrawn); the notice is unchanged when nothing is hidden. The clipping rule and its notice see only the compartments left on, so a hidden compartment is neither drawn nor counted as clipped; the rendering's showCompartments = false still folds nothing.
  • showReferences is the switch of the ref rows wherever they are listed (Cameo's references compartment), showParts of the composite part rows; an attribute row, referential by Usage::isReference, is an attribute row.
  • Mermaid, PlantUML and D2 write compartments from the same folded Node.Compartments, so they follow with no change of their own; the text form never settled unplaced nodes and lists the members as before (documented in view-rendering-forms.md).
  • The switches act where compartments are drawn: the positioned tree (asTreeDiagram, what a Cameo BDD migrates to). The interconnection, state, action and case renderings fold no members into compartments, so a switch written on a symbol of those views (an IBD part's showItems) has nothing to hide and changes nothing there.
  • With an MTIP -layout export, a stream symbol's switches move onto the export's box of the same element when the element's symbols agree on them; symbols of one element that differ leave the export's box as it is.

Behaviour changes

  • The folding notice gains a form for the hidden case (above). Every existing golden keeps its wording: no existing renderer golden moved, no existing migrate golden moved (no fixture stream carried a SUPPRESS_* value).
  • New goldens: internal/ir/view/testdata/suppressed-compartments-{selective,collapsed,cramped,roomy,bare}.{dot,mermaid,plantuml,d2,text}.golden (25) and tests/migrate/testdata/xmi/suppressed_compartments.golden.{sysml,report.txt} (2), with the fixtures suppressed-compartments.sysml, suppressed_compartments.{xmi,stream.xml,layout.xml}.
  • The transformation census baseline moves by the one added fixture (124 fixtures; uml:Class 570, uml:Property 571 on current develop) — re-measured, no mapping class changes.

Measured on SEM.mdzip (develop vs this head)

  • Switches written: 868 on 261 of 428 placed symbols (showActions 225, showItems 208, showPorts 141, showAttributes 100, showParts 66, showReferences 64, showConstraints 64); unsupported, dropped: SUPPRESS_BEHAVIORS 202, SUPPRESS_STRUCTURE 168, SUPPRESS_SIGNAL_RECEPTIONS 168, SUPPRESS_IBD_BEHAVIOR_COMPARTMENT 63.
  • Model text: stripping the switch clauses from this head's output gives develop's output byte for byte; under -portable the output differs only by the switch clauses and the inlined DiagramLayout library text (the seven attributes and their doc comment).
  • -render-all -render-form dot, 34 views: compartment rows reported as clipped 386 → 356 over the same 18 views. Per requested view (clipped rows before → after): Imaging BDD 26 → 23 (3 rows hidden), Subsystem Architecture 120 → 112 (8), Expanded Domain Model - Part 1 20 → 13 (7), Logical Structure 55 → 54 (1). On SEM the change is in the notices, not the picture: the Graphviz PNGs of all four views are byte-identical before and after, because every row the modeler collapsed lay in the clipped-off part of its box already; the notices now report those rows as the modeler's choice (… in compartments its Layout hides left undrawn) instead of as rows without room. The view fixture's roomy golden shows the visible case: a box with room for every row lists everything but the collapsed parts.
  • sysml -validate: clean. Pilot validate-sysml-batch on the -portable output: clean.
  • Follow-up for the maintainer, not changed here: on Cameo's 200×33 'Imaging Subsystem' bar the rows still clipped are parts Cameo shows in the model but draws as composition lines rather than compartment rows; the parts compartment is not suppressed on that symbol, so the fit-and-clip rule applies to them as before. Whether a part drawn as a line should also fold into its owner's compartment is a separate decision.

Specification basis

SysML v2 Language Specification (the version the pinned 2026-08 pilot implements), Metadata (§7.25): user-defined metadata definitions whose attribute values are model-level evaluable; DiagramLayout is the non-normative OpenSysML extension library (docs/project/diagram-layout-annotations.md). No row of docs/project/spec-compliance.md moves.

How it was verified

  • go build ./..., go vet ./..., gofmt -l . clean; go test ./internal/translate/xmi/sysmlv1/ ./internal/translate/migrate/ ./internal/ir/view/ ./internal/semantic/semantics/ ./tests/migrate/ pass against the re-pinned DiagramLayout (Open-MBEE/OpenSysML-Extensions-Library 5134a14, vendored with scripts/sync-extension-libraries.sh; stdlib.snapshot regenerated, pilot-diff baseline re-recorded for the library digest with verdicts unchanged). The re-pin moves the vendored copy from the previous pin to upstream main, so it also carries Route.general from feat(DiagramLayout): Route.general names the general a specialization edge is routed to OpenSysML-Extensions-Library#7; the renderer and semantic support for that attribute is fix(view): route specialization edges from Cameo and keep the keyword line in a stated box #1166's, not this PR's, and nothing here reads it.
  • New tests: TestReadSymbolsCompartmentsInheritStyle (style chain, unvalued default, own-over-inherited), TestHiddenCompartments (kind map, all-suppressed rule, unsupported), TestGoldenSuppressedCompartments (migrate, with errors() validation) and TestSuppressedCompartmentsSurviveAnExportRecord (MTIP join), TestGoldenSuppressedCompartments and TestSuppressedCompartmentsAreNotListedNorClipped (view: one kind off, all off, a cramped box, a roomy box, a rendering without compartments).
  • transformation-census -check, scripts/check-doc-ids.py, scripts/changelog.py check, make docs-check clean.
  • SEM.mdzip measured as above.

Checklist

  • make test and make lint pass locally
  • Tests added or updated for the change
  • Documentation extended where it already covers the surface (see CONTRIBUTING.md)
  • Changelog entry added as changes/unreleased/<slug>.<section>.md, not as an edit to CHANGELOG.md
  • baselines regenerated and make docs-counts run if a gate count moved (compliance rows need nothing: the census is counted at docs build)
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/4dd157ac12a04bf69f2b84e455f8a37b
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/4dd157ac12a04bf69f2b84e455f8a37b?variant=devin
Requested by: @HuiJun

…agramLayout show… switches

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration Bot and others added 2 commits October 16, 2026 05:30
… and the kind map

Co-Authored-By: jason.han <hanhuijun@gmail.com>
… count them apart from the clipped rows

The migrator writes the switches from a Cameo symbol's own and inherited
SUPPRESS_* properties; the positioned-tree renderer neither lists a hidden
compartment's rows nor counts them among the rows the box's stated size
clips. Fixtures: a view fixture (one kind off, all off, a cramped box, a
rendering without compartments) and a minimal Cameo stream with a style
chain.

Co-Authored-By: jason.han <hanhuijun@gmail.com>

@devin-ai-integration devin-ai-integration Bot left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

Devin Review found 3 potential issues.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

Comment thread internal/translate/migrate/streams.go
Comment thread internal/translate/migrate/diagrams.go
Comment on lines +149 to +152
if row.Referential && !row.StandIn {
return node.Geometry.Hidden.Hides("references")
}
return node.Geometry.Hidden.Hides(compartmentName(row.Kind))

@devin-ai-integration devin-ai-integration Bot Oct 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🟡 Suppressed kinds leave reference rows visible

When showParts = false, hiddenRow leaves a ref part visible unless showReferences is also false. The reference check bypasses the row's kind, so suppressed kind compartments still show referential members.

Learn more

A tree can put a ref part in the parts compartment, as compartmentsOf groups rows by keyword. The new reference check prevents the kind check from running. It therefore keeps referential members of a hidden kind in the box when references remain enabled.

Example: With showParts = false and showReferences unset, ref part mount : Mount remains listed in parts, while an ordinary part lens : Lens is removed.

Recommended fix: Hide a referential row when either its references switch or its own kind's switch hides it; retain the kind check after the reference check.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Intended, and documented in docs/project/diagram-layout-annotations.md: showReferences is the switch of the ref rows wherever they are listed, showParts the switch of the composite part rows. The switches mirror the tool's compartments: Cameo lists a reference property in its own "references" compartment, apart from "parts", and SUPPRESS_PARTS_COMPARTMENT leaves the references visible there. A ref part in a box whose Layout says showParts = false with showReferences unset is therefore still listed, as it was in the source diagram; a modeler who collapsed both gets showReferences = false too. Hiding it under showParts would drop a row the diagram showed.

One correction did come out of checking this: an attribute is referential by Usage::isReference, so the first revision treated attribute rows as reference rows and showAttributes = false did not hide them. referenceRow now counts only structural non-composite usages (ref part, ref item, …) as reference rows (2d43de8).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Handled in 96e0ce3. The rule is deliberate: a ref part belongs to the references compartment (SysML v2 graphical notation; Cameo's SUPPRESS_REFERENCES_COMPARTMENT is what hides it there), so showReferences governs it and showParts does not — otherwise a Cameo symbol that suppresses parts but shows references would lose its reference rows. What 96e0ce3 fixed is the over-wide classification: referenceRow now marks only structural non-composite usages as referential, so attribute rows (which Usage::isReference also reports) follow showAttributes. Covered by TestGoldenSuppressedCompartments and TestSuppressedCompartmentsAreNotListedNorClipped (internal/ir/view; the selective view shows a ref part under showParts = false and hides it under showReferences = false).

devin-ai-integration Bot and others added 2 commits October 16, 2026 05:51
…t places it; an attribute row is not a reference row

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…nt it gathers is suppressed

Co-Authored-By: jason.han <hanhuijun@gmail.com>

@devin-ai-integration devin-ai-integration Bot left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

Devin Review found 1 new potential issue.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

Devin Review

Comment on lines +124 to +126
for _, p := range stream.Placements {
if _, ok := compartments[p.ID]; !ok && (len(p.HiddenCompartments) > 0 || len(p.UnsupportedCompartments) > 0) {
compartments[p.ID] = p

@devin-ai-integration devin-ai-integration Bot Oct 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🟡 Repeated symbols hide the wrong compartments

When repeated symbols share an element ID, compartments can take suppressions from a later symbol for the first export placement. The first symbol's visible rows then disappear from its box.

Learn more

A diagram can contain more than one symbol for the same model element. The symbol stream records each symbol separately in streamRecord, but the export and merge pair placements only by element ID. The new map skips placements without suppressions, so the next symbol with that ID can supply switches for a different box. placements then writes the first placement for that reference and silently drops later placements.

Example: Two Camera shapes share _blk_camera. The first has no suppressed compartments and is placed at x=50 by the export; the second suppresses parts. The merged layout at x=50 receives showParts = false, hiding the first shape's parts.

Recommended fix: Match a stream symbol to its exported box before transferring settings, using geometry or a stable symbol identifier where available. If matching is ambiguous, avoid borrowing settings from another symbol; test multiple symbols of one element with different switches.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: the merge now transfers a stream symbol's switches onto the export's box only when every symbol of that element in the stream agrees on them (HiddenCompartments and UnsupportedCompartments equal); symbols of one element that differ leave the export's box as the export states it, so no box borrows another symbol's settings. TestSuppressedCompartmentsSurviveAnExportRecord now gives Lens two symbols that differ and an export box: the box keeps the export geometry and no switch, while the single-symbol Camera keeps its four.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f43e8fb (a symbol's switches reach an export's box only when every stream symbol of that element states the same hidden kinds; disagreeing symbols transfer nothing) and 66e3204 (agreement judged on the hidden kinds alone, so differing unsupported properties no longer discard agreed switches). Covered by TestSuppressedCompartmentsSurviveAnExportRecord (internal/translate/migrate) and the second Lens symbol in tests/migrate/testdata/xmi/suppressed_compartments.stream.xml, checked by TestGoldenSuppressedCompartments.

… box only when the element's symbols agree

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 5 commits October 16, 2026 05:58
…hidden kinds alone

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
…ment left out

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…ment switches

Re-vendors OpenSysML Libraries from Open-MBEE/OpenSysML-Extensions-Library
5134a14 (Layout gains showAttributes, showParts, showReferences, showPorts,
showConstraints, showItems, showActions), regenerates stdlib.snapshot and
re-records the pilot-diff baseline's library digest; verdicts unchanged.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…ameo-suppressed-compartments

Co-Authored-By: jason.han <hanhuijun@gmail.com>

# Conflicts:
#	docs/project/sysml-v1-transformation-census-baseline.json
#	docs/project/sysml-v1-transformation-census.md

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

* general it is drawn to, `ref :>> general : Sup;`, one Route
* per general. */
attribute points : ScalarValues::Real[0..*] ordered nonunique;
ref general : Base::Anything[0..1];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🔴 Specialization routes never reach the drawing

When a model supplies Route.general for a specialization, the tree draws its line without those waypoints. edgesOf creates relationship edges without reading routes, so each general-specific path is lost.

Learn more

The new general feature identifies which target a Route on a specific element belongs to. readRoute collects only points, while RouteOf selects a single annotation per element. edgesOf never requests a Route for its specialization edges, so it cannot use even one set of waypoints.

Example: A tree draws Child :> ParentA, ParentB with a Route about Child for each parent. Both lines remain automatically laid out instead of following their separate point sequences.

Recommended fix: Resolve Route sites by view and the general target, then attach the matching waypoints to each structural relationship edge in treeGraph.edgesOf. Retain the existing Route behavior for connections and other edges with no general.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not this PR's change. The re-pin moves the vendored copy from the previous pin to upstream main, which also carries Route.general from Open-MBEE/OpenSysML-Extensions-Library#7. The semantic and renderer support for it (LayoutSite.General, Model.SpecializationRouteOf, the tree giving each specialization edge its Route) is #1166's, still open against develop; nothing in this PR reads general, and without #1166 the attribute is an unset optional feature, so no behaviour moves here. Leaving the thread open for the maintainer to decide the merge order with #1166.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed cause and resolution: the re-pin brought Route.general into the vendored library while #1166's consumer code (Model.SpecializationRouteOf, LayoutSite.General, the tree's per-general routing in internal/ir/view) was not yet on this branch — #1166 merged into develop at 325735f, after this branch's previous develop merge (81777ca). Develop 325735f is now merged into this branch, so the consumer code is present; no code change in this PR.

…ameo-suppressed-compartments

Co-Authored-By: jason.han <hanhuijun@gmail.com>

# Conflicts:
#	docs/project/pilot-differential-baseline.json
#	internal/workspace/libs/stdlib.snapshot
#	scripts/extension-libraries-pin.sh
@devin-ai-integration
devin-ai-integration Bot marked this pull request as ready for review October 16, 2026 14:06
@HuiJun
HuiJun merged commit 2b3cf49 into develop Oct 16, 2026
27 checks passed
@HuiJun
HuiJun deleted the devin/1792127253-cameo-suppressed-compartments branch October 16, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant