feat(graph): graph-based ontology editing#201
Closed
dgokeeffe wants to merge 13 commits into
Closed
Conversation
…ge graph Rewrite the knowledge graph visualization with W3C RDF/SKOS-native rendering and add inline editing capabilities — no more navigating away from the graph. Graph infrastructure: - Rebuild ForceGraph2D with semantic edge types (skos:broader, rdfs:subClassOf, owl:equivalentClass, skos:related, rdf:type, rdfs:subPropertyOf) - Add concept detail panel with W3C-native property display - Add edge filter toggles and graph search overlay - Surface ontology structure (Concept vs Property vs Scheme node types) Graph-based editing (Phase 1-3): - Click node → inline detail panel with edit/delete actions - Right-click node → context menu (View, Edit, Create Link, Delete) - Right-click background → Create New Concept - Click-to-connect link draw mode with visual indicators - All editing gated behind semantic-models READ_WRITE permission - Reuses existing CRUD endpoints, no backend changes needed Co-authored-by: Isaac
dgokeeffe
force-pushed
the
feat/agl-energy-demo
branch
from
0f6ab97 to
ed77d73
Compare
Author
Test EvidenceAll features verified manually via Chrome DevTools MCP on Phase 1: Detail Panel (Click to Inspect)
Phase 2: Context Menu + Creation
Phase 3: Click-to-Connect (Link Draw Mode)
Regression Checks
Tested by Isaac via Chrome DevTools MCP |
Author
ScreenshotsScreenshots captured during testing (available on request):
|
Author
|
Collaborator
larsgeorge-db
left a comment
larsgeorge-db
left a comment
There was a problem hiding this comment.
Hey Dave, thank you for the work! I have a few questions/remarks at this point:
- Why switch away from Cytoscape as the renderer? We used ReactFlow2d in the past but then switched to Cytoscape as it has more options available
- Editing triples is disabled for internal ontologies. We need to still discuss how to handle point or full updates of those, but the UI otherwise makes them read-only. So while you can show the node details, the Edit feature should be more deliberate.
- The edit dialog has the wrong Z-order, it shows under the dimmed overlay of the main view. You can also see this from your own screenshot.
- The visual style used is different from the rest of the app. Would be good to align this with the main one.
Contributor
|
Rebasing this branch to capture the changes to transitive packages and trigger CI. |
will-yuponce-db
force-pushed
the
feat/agl-energy-demo
branch
from
ed77d73 to
5bf6f72
Compare
dgokeeffe
force-pushed
the
feat/agl-energy-demo
branch
from
5bf6f72 to
ed77d73
Compare
added 11 commits
May 5, 2026 17:02
Resolves the diverging-base situation between PR databrickslabs#201 and main, and addresses review feedback on the renderer choice. - Takes main's Cytoscape knowledge-graph.tsx instead of the ForceGraph2D rewrite, preserving showDomainBoxes (compound nodes), language-aware label resolution (selectedLanguage / resolveLabel), and the existing semantic styling. Reverts the Cytoscape→ForceGraph2D regression flagged in PR databrickslabs#201 review. - Ports the editing UX onto Cytoscape via cy.on() handlers: - cxttap on .concept-node → onNodeRightClick (context menu) - cxttap on canvas → onBackgroundRightClick (create-new menu) - tap branches on linkDrawSource: clicking another node calls onLinkDraw, otherwise normal onNodeClick - background tap and Escape key both cancel link-draw mode - new .link-draw-source stylesheet entry: pink dashed ring on the source - Drops react-force-graph-2d from package.json (no remaining consumers). - Regenerates yarn.lock from main. The renderer-agnostic editing components (concept-detail-panel, graph-context-menu, edge-filter-toggles, graph-search-overlay) are unchanged. Co-authored-by: Isaac
…slabs#201) The original PR's link-draw banner ("Click a target node to create link from <source>" with pulsing indicator and Cancel X) lived inside the ForceGraph2D rewrite of knowledge-graph.tsx. Reverting that file to main's Cytoscape version dropped the banner along with the renderer. Restored as an overlay in <GraphTab>, where linkDrawSource is already in scope. Renderer-agnostic — no Cytoscape coupling. Verified via Chrome DevTools MCP against a live ontos backend with 247 concepts: right-click → Create Link → banner renders with resolved source label, X button calls onLinkDrawCancel, Escape and X both clear the .link-draw-source visual treatment on the source node. Co-authored-by: Isaac
After the initial port from PR databrickslabs#201's ForceGraph2D rewrite onto main's Cytoscape, two regressions were caught via Chrome DevTools MCP: 1. Right-click in fullscreen mode did nothing — handlers only wired to the main cyRef, not the separate fullscreenCyRef instance that <KnowledgeGraph> creates inside its <Dialog>. 2. The link-draw banner (added in 1b1e105 in <GraphTab>) sat at the top of the GraphTab container and visually overlapped the source-filter legend chips above the graph canvas, AND was hidden behind the fullscreen Dialog overlay when in fullscreen mode. Fixes: - Extract event-wiring into wireCyHandlers(cy: Core), called from BOTH the main and fullscreen <CytoscapeComponent> cy-mount callbacks. - Use a handlersRef to keep wireCyHandlers referentially stable; that way listeners don't churn on every prop change. - Apply .link-draw-source class to BOTH cy instances in the visual treatment effect, with isFullscreen in deps so the new fullscreen cy gets the highlight on open. - Move the link-draw banner from <GraphTab> into <KnowledgeGraph> as a small <LinkDrawBanner> helper, rendered inside each cy container's relative wrapper. Result: banner sits over the canvas (not above it), and shows correctly in both regular and fullscreen views. Verified end-to-end via Chrome DevTools MCP against a live ontos backend with 247 concepts: - Right-click in regular view → context menu (4 items) - Right-click in fullscreen view → context menu (4 items) - Create Link From... in either view → banner over canvas + pink dashed ring on source node - Click target node in either view → "Create Link" dialog opens with source/target pre-filled and skos:broader default - Escape and X-button both clear banner and source class Co-authored-by: Isaac
Addresses PR databrickslabs#201 review point 3 — edit dialog appeared under the dimmed overlay when opened from inside the fullscreen knowledge graph. Root cause: shadcn's Dialog defaults to z-50 for both Content and Overlay. Two open Dialogs (the fullscreen graph and the edit dialog) collided at z-50, with stacking decided by Portal mount order. From within fullscreen the edit dialog's overlay sat under the fullscreen content, hiding the form behind a dim layer. Fix: - Add an optional `overlayClassName` prop on `<DialogContent>` so the underlying `<DialogOverlay>` can be elevated when nested. Default behavior unchanged for all existing dialog usages. - Concept editor and link editor pass z-[60] for both content and overlay, putting them cleanly above the fullscreen Dialog's z-50. Verified via Chrome DevTools MCP: opened fullscreen, right-clicked a node, clicked Edit Concept — edit dialog renders above a dim overlay that correctly covers both the main view and the fullscreen graph. Co-authored-by: Isaac
Addresses PR databrickslabs#201 review point 2 — Lars: "Editing triples is disabled for internal ontologies. The Edit feature should be more deliberate." The backend already gates writes on collection.is_editable (see update_concept at semantic_models_manager.py:3208), but the frontend was offering Edit/Delete/Create-Link buttons that would fail server- side. Frontend now mirrors the gate. How it works: - Build a Map<source_context_suffix, is_editable> from the collections state already loaded by OntologyHomeView. Concepts carry source_context with the prefix stripped ("databricks_ontology"), while collections expose the full IRI ("urn:taxonomy:databricks_ontology") — index on the bare suffix so they line up. - isConceptEditable(concept) returns false when the matched collection has is_editable=false. Unknown sources fall through to canWrite (the backend remains the final guard). - GraphContextMenu and ConceptDetailPanel both consume the helper: Edit / Delete / Create-Link callbacks become undefined for concepts from read-only collections, so the menu items / detail-panel buttons simply don't render. - Background right-click ("Create New Concept") and the toolbar "+ New Concept" button are unaffected — they create concepts in a user-selected editable collection. Verified via Chrome DevTools MCP: - Right-click a concept from urn:taxonomy:odcs-ontology (is_editable= false) → context menu shows only "View Details" - Background right-click still shows "Create New Concept" Co-authored-by: Isaac
Addresses PR databrickslabs#201 review point 4 — Lars: "The visual style used is different from the rest of the app." Two specific divergences from the rest of the app: 1. Custom slide-in panel via fixed-positioned divs + a hand-rolled bg-black/20 overlay, while the rest of the app (e.g. <CommentSidebar>) uses shadcn <Sheet>. Different animation, different overlay opacity, different keyboard/focus behavior. 2. Type badges on the panel header used inline rgba/hex values (rgba(139,92,246,0.12), #8b5cf6, etc.), bypassing the Tailwind palette and dark-mode tokens used elsewhere. Changes: - Wrap the panel content in <Sheet>/<SheetContent side="right">. Removes the custom overlay, manual close X (Sheet provides one), and z-index juggling — Sheet's portal + animation are handled by Radix. - Move Edit/Delete buttons to right-12 to leave Sheet's standard right-4 close-button slot free. - Replace W3C_TYPE_STYLE (inline rgba/hex) with W3C_TYPE_BADGE_CLASS and W3C_TYPE_STRIPE_CLASS that map to Tailwind utility classes (bg-emerald-500/10, text-blue-500, etc.). Theme-aware, no inline styles, identical visual outcome. Verified via Chrome DevTools MCP — panel mounts as Sheet, type badge rendered with classes (no inline style attribute), stripe colored via Tailwind class, Sheet's bg-black/80 overlay matches the app's other slide-in panels. Co-authored-by: Isaac
The Sheet conversion in 8da7956 surfaced two Radix accessibility warnings (Sheet wraps Dialog primitives, which require a Title):⚠️ DialogContent requires a DialogTitle for the component to be accessible for screen reader users.⚠️ Missing `Description` or `aria-describedby={undefined}` for {DialogContent}. Fixes: - The visible H1-style concept name is now a <SheetTitle> with the same Tailwind classes (text-xl font-bold pr-8 leading-tight). The base SheetTitle's text-lg font-semibold is overridden by tw-merge, so the visual is unchanged. - The loading state ("Loading...") gets a sr-only <SheetTitle> so the Sheet always has a screen-reader-discoverable name. - Set aria-describedby={undefined} on <SheetContent> to opt out of the description requirement — there's no single sentence that fits a panel scrolling through synonyms, definitions, hierarchies, etc. Verified via Chrome DevTools MCP — both warnings cleared, panel still mounts as Sheet with aria-labelledby pointing at the SheetTitle, X close still functional, open/close cycles clean. Co-authored-by: Isaac
In-flight edits in <ConceptEditorDialog> were pure React state — closing
the dialog (Cancel / Escape / X) or refreshing the page silently lost
everything. Adds two layers of safety:
1. **sessionStorage autosave**: every field change writes the form to
sessionStorage under a per-concept key
(ontos.concept-editor.draft.<iri-or-new:collection>). Survives page
refresh and back/forward nav; cleaned up on tab close (so drafts
don't accumulate). On dialog open, an existing draft is restored
transparently.
2. **Confirm-discard guard**: closing the dialog with unsaved changes
pops a native confirm("Discard unsaved changes?"). Skipped during
page unload (Radix fires onOpenChange on unmount; the browser's own
beforeunload UX handles tab close instead).
Subtle ordering bugs caught in test:
- The existing [concept] effect (deps: concept, collection, open)
re-fires every time the dialog opens and was clobbering the
restored draft because it ran later in declaration order. Fixed by
short-circuiting that effect when a draft exists for the current
draftKey.
- Autosave on the same render that restores would otherwise see stale
formData against the freshly-updated cleanSnapshotRef and overwrite
the draft with empty data. Guarded with skipNextAutosaveRef.
Verified via Chrome DevTools MCP:
- Type in label → sessionStorage populated immediately
- Reload page → storage survives, no spurious confirm prompt
- Reopen dialog → label pre-filled, storage NOT overwritten with empty
- Click Cancel on dirty form → confirm fires, accept clears draft and
closes, dismiss keeps dialog open
Co-authored-by: Isaac
…tions Following PR databrickslabs#201 review feedback comparing visual feel to the original ForceGraph2D rewrite. Adds polish to the existing Cytoscape view to get closer to that aesthetic without changing renderer: - **Animation transitions**: nodes and edges now declare `transition-property` + `transition-duration` (200ms / 180ms ease-out) so hover/select state changes scale smoothly instead of snapping. - **Drop shadows on concept nodes**: 8px blur in the node's own color, alpha tuned per theme (0.5 dark, 0.25 light). Reads as a soft glow on dark, a soft drop on light. - **Hover halo**: overlay-color/overlay-opacity/overlay-padding produces an outward color ring around the hovered node — the characteristic "force graph" hover-glow effect. - **Selected halo**: same overlay treatment but in gold (#FFD700) for the selected state, building on the existing gold ring. - **Property nodes** get matching hover/selected halos. - **Edge transitions**: width / opacity / color animate on hover instead of snapping to gold. - **Cose layout tuning**: longer ideal edges (80 → 110), more repulsion (400k → 600k), more iterations (1000 → 1500), gentler cooling (0.95 → 0.96), eased animation. Result is more breathing room and a more organic settled-look. - **Slight node-size bumps**: concept 24 → 26, property 20 → 22 base; hover sizes scale up proportionally. Polish is most visible at interactive zoom levels (clusters / single- node neighborhoods). At fit-to-screen with 247 concepts the nodes are still small dots — that's a function of node count, not styling. Co-authored-by: Isaac
Adds cytoscape-fcose ("fast compound spring embedder") as a layout
extension and makes it the default in place of the built-in cose. Same
underlying force-directed model, but with substantially better default
aesthetics, faster convergence, and gentler handling of compound
domain nodes — closer to the look of dedicated force-graph libraries
without leaving Cytoscape.
Concrete changes:
- package.json: add cytoscape-fcose ^2.2.0 (run `yarn install` to fetch).
- knowledge-graph.tsx:
- Register the extension via cytoscape.use(fcose). The call is
idempotent so HMR / Strict-mode double-mounts are safe.
- Extend LayoutType union with 'fcose' and add 'fcose' as the default
initial layout state. Existing 'cose' remains selectable as
"Force-Directed (legacy)" for comparison.
- Add fcose case to getLayoutConfig with sensible parameters:
quality='proof' (full convergence), randomize=false (use current
positions for stable re-layouts), animate='end' (single transition
to final positions), nodeRepulsion=8000, idealEdgeLength=90,
edgeElasticity=0.45, gravity=0.25, packComponents=true.
Build / install note: the dependency is registry-neutral — package.json
declares the version range only, and yarn.lock will resolve it via
whatever registry yarn sees at install time. Locally you can use the
internal proxy (`yarn config get registry`); CI and Databricks Apps
deploys use the default public registry.
Co-authored-by: Isaac
The first fcose config produced a degenerate diagonal stripe inside each compound domain — caused by tile=false + randomize=false + quality='proof' interacting badly with showDomainBoxes. Fixed by returning to fcose's natural defaults for compound handling: - tile=true (default): pack compound children naturally instead of letting them snake corner-to-corner - randomize=true: let fcose pick a fresh starting state per layout run so it doesn't lock onto whatever the previous layout left behind - quality='default': converges fast enough on ~250 nodes; 'proof' was over-converging into local minima for our compound structure - nodeRepulsion 8000 → 4500: less aggressive separation gives tighter intra-domain clustering - Added tilingPaddingVertical/Horizontal=12 for breathing room between tiled children inside each domain box Also locks in the yarn.lock entries for cytoscape-fcose ^2.2.0 and its transitives (cose-base, layout-base). The lock URLs all point at the public registry (registry.yarnpkg.com), keeping CI/deploy builds registry-neutral as discussed. Verified visually via Chrome DevTools MCP: each domain compound contains an organic force-distributed cluster of its concept nodes; inter-domain edges visible as faint connecting lines. Co-authored-by: Isaac
Author
|
Hey Lars — thanks for the review. Each point addressed below; commits referenced are on this branch ( 1. Why switch away from Cytoscape? → revertedYou're right — went with Reverted to Cytoscape in Subsequent fixes:
I also went a step further on the look since "doesn't look as nice as before" was the implicit fairness check:
2. Editing should be more deliberate for internal ontologies → doneBackend already gates writes on
For internal taxonomies ( 3. Edit dialog Z-order → fixedRoot cause was two
Verified via Chrome DevTools: opening Edit from inside fullscreen now puts the form above a dim overlay that correctly covers both the main view and the fullscreen graph. Bonus improvement layered on top — 4. Visual style alignment → doneTwo specific divergences:
Fixed in
If there are more specific style divergences you spotted, happy to address — these were the most obvious ones. Plus: editor draft persistence + confirm-discard guard (
|
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
3 participants
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.
Summary
All editing actions are gated behind
semantic-modelsREAD_WRITE permission. No backend changes — reuses existing CRUD endpoints (POST/PATCH/DELETE /api/knowledge/concepts/{iri}).Files changed
ontology-home.tsxconcept-detail-panel.tsxonEdit/onDeleteaction buttons in panel headerknowledge-graph.tsxonNodeClickordering buggraph-tab.tsxgraph-context-menu.tsxTest plan
semantic-modelswrite permission) → no New Concept button, no edit/delete in context menu or panelThis pull request was AI-assisted by Isaac.