Merge vertex styles across all of a vertex's types instead of picking one arbitrarily - #2138
Open
giantamoeba wants to merge 1 commit into
Open
Merge vertex styles across all of a vertex's types instead of picking one arbitrarily#2138giantamoeba wants to merge 1 commit into
giantamoeba wants to merge 1 commit into
Conversation
… one arbitrarily For a SPARQL/RDF connection, a resource with more than one rdf:type (common under RDFS/OWL inference, where every ancestor class is asserted as a peer rdf:type triple) had its styling/displayNameAttribute resolved from whichever type happened to come first in SPARQL result order -- unspecified by the SPARQL spec, and in practice often the most generic ancestor class rather than the specific one a user actually styled. Adds resolveVertexStyleForTypes/mergeVertexStyleFields, which fold a vertex's full type set (sorted for a stable, reproducible tiebreak) into one merged style instead of keying off a single primaryType. Wires this through useGraphStyles.ts (Cytoscape stylesheet generation), displayVertex.ts (displayNameAttribute resolution), renderedEntities.ts (the type key used as the Cytoscape selector), and VertexRow.tsx (search result styling). See PR_DRAFT.md for the full writeup and reproduction.
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.
Description
Fixes the multi-type vertex styling bug described in #2134.
Today, when a vertex has more than one type — routine for SPARQL/RDF connections under RDFS/OWL inference, where every superclass gets asserted as a peer
rdf:type— styling (icon, color, shape, border, etc.) and thedisplayNameAttribute/longDisplayNameAttributelookup are resolved from a single "primary type," whichcreateVertexsets totypes[0]. For Gremlin/openCypher that's a reasonable choice (labels are already an ordered list from the graph engine); for RDF,rdf:typeis an unordered set, so "first" is really "whatever order the SPARQL engine happened to return," which is not guaranteed by SPARQL and in practice is often the most generic ancestor class — so a user's styling for the specific class they actually care about silently never applies.This PR replaces the single-type lookup with a merge across every one of a vertex's types, generalizing the resolution model already described in #2003 (
{ ...appDefault, ...userCustom.get(type) }for one type) to fold over multiple types:core/StateProvider/graphStyles.ts: addsvertexTypeSetKey(a stable, order-independent identity for a vertex's full type set),mergeVertexStyleFields(folds each type's stored style into one, field by field, so a field unset by one type falls through to another that sets it — e.g. aborderColorstyled once on a shared ancestor class applies to every subtype automatically), andresolveVertexStyleForTypes(the merged result overlaid onappDefaultVertexStyle, same shape as the existingresolveVertexStyle). Also addsvertexStyleAtom.getForTypes,vertexStyleByTypesAtom, and theuseVertexStyleForTypeshook.core/StateProvider/displayVertex.ts:displayVertexSelectornow resolvesdisplayName/displayDescriptionviaresolveVertexStyleForTypes(vertexTypes, ...)instead ofvertexStyleByTypeAtom(vertex.type).core/StateProvider/renderedEntities.ts: a rendered vertex's Cytoscapedata.type(used for stylesheet selector matching) is nowvertexTypeSetKey(vertex.types)instead ofvertex.primaryType.modules/GraphViewer/useGraphStyles.ts: builds stylesheet rules for the distinct type-sets actually present among rendered vertices, in addition to (not instead of) the existing one-rule-per-schema-type generation — so single-typed vertices are unaffected (vertexTypeSetKeyof one type is that type itself) and multi-typed vertices get their own correctly-merged rule instead of colliding with one arbitrary member type's rule.components/VertexRow.tsx: search-result rows now resolve style viauseVertexStyleForTypes(vertex.types)instead ofuseVertexStyle(vertex.primaryType).Not changed: anywhere a single, real schema type is genuinely what's wanted — the Legend panel, the Schema view's per-type Styles editor, the filter sidebar,
VertexIconByType/VertexSymbolByType— all still useuseVertexStyle(type)/vertexStyleByTypeAtomunmodified, since those show or edit one type's own style, not a specific multi-typed vertex instance.Conflict resolution — an honest tradeoff
When two of a vertex's types both set the same field (e.g. both set
color), something has to win. I resolve this by folding the vertex's types in ascending lexicographic order, so the lexicographically-last type wins conflicts. This is a stable, reproducible tiebreak, not a specificity judgement — nothing in the app currently tracksrdfs:subClassOf(or any other) class hierarchy; schema sync only samples instance data (types + attribute names) for every connector. There's no signal available today to determine which of a resource's asserted types is actually "more specific." Sorting at least makes the outcome a deterministic property of the type names themselves instead of an accident of query result order, which is what motivated this fix in the first place.A hierarchy-aware tiebreak (prefer the type that is not an
rdfs:subClassOfancestor of any other asserted type) would be a strictly better conflict resolution and a natural follow-up once class-hierarchy data is tracked anywhere in the app — happy to discuss whether that's in scope for a first pass or worth its own follow-up issue.Validation
pnpm run check:types— passespnpm run check:lint— 0 warnings, 0 errorspnpm run check:format— passespnpm run test— full suite passes (one pre-existing, unrelated failure insafeSessionStorage.test.tsreproduces identically onmainwith no changes — avi.mocked(logger.warn)assertion that appears environment-dependent, not something this PR touches)graphStyles.test.tscoververtexTypeSetKey(order-independence, dedup, distinctness),mergeVertexStyleFields(non-conflicting merge, conflict tiebreak order-independence, missing styles, empty result), andvertexStyleAtom.getForTypes(merge-overlaid-on-defaults, order-independence)useAllRenderedVertexStylesfed an unstable array reference (fromuseDisplayVerticesInCanvas()) intouseMemo, defeating memoization — on a graph with many vertices sharing type combinations this showed up as a pathological re-render storm during session restore. Fixed by readingnodesAtomdirectly and memoizing on a content-based string key over the distinct type-set combinations actually present, rather than on the vertex list's object identity. Covered by two new regression tests inuseGraphStyles.test.tsx: one asserting the returned array is referentially stable (===) across a no-op re-render, and one asserting it stays stable when a new vertex reuses an existing type combination but correctly rebuilds when a genuinely new combination appears.rdf:typetriples): before this fix, styled classes rendered with the app's default icon/color depending on triple-store query order; after, they render with the styling set on their specific class.ex:Specificsets icon+color,ex:Generalsets shape+border), same styles file, loaded into an unmodified v3.2.2 build and this branch's dev build, both pointed at the same Fuseki dataset:Related Issues
rdf:typewhen a resource has multiple types (not a merge of all matching styles) #2134Check List
pnpm checkspasses with no errors.pnpm testpasses with no failures. — 2677/2678 pass; the one failure (safeSessionStorage.test.ts,resolveSessionStorage > warns and falls back to in-memory storage when sessionStorage is unavailable) reproduces identically on unmodifiedmain, so it's pre-existing and unrelated to this change, not something this branch introduced.graphStyles.test.ts(vertexTypeSetKey,mergeVertexStyleFields,vertexStyleAtom.getForTypes) and two regression tests inuseGraphStyles.test.tsxfor the memoization fix.docs/adr/for anything describingprimaryType/single-type style resolution; nothing references it, so nothing needed updating.