Skip to content

feat(ontology): add Cohort Discovery rule deletion, fix stale URIs and SWRL raw-editor sync #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jeremiaspf wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat(ontology): add Cohort Discovery rule deletion, fix stale URIs and SWRL raw-editor sync #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/front/static/ontology/js/ontology-axioms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,15 @@ window.AxiomsModule = {

select.innerHTML = '<option value="">Select...</option>';
items.forEach(item => {
select.innerHTML += `<option value="${item.uri || item.name}">${item.name}</option>`;
// Prefer the plain name over the class/property's stored `uri`.
// That `uri` field can be stale (e.g. left over from before the
// ontology's base URI was renamed) — sending just the name lets
// the backend (OntologyGenerator._resolve_uri) always rebuild
// the correct, current URI from `base_uri + name`, the same way
// every other part of the ontology already resolves class/
// property identities. Falls back to `uri` only if a name is
// somehow missing.
select.innerHTML += `<option value="${item.name || item.uri}">${item.name}</option>`;
});
},

Expand Down Expand Up @@ -131,7 +139,9 @@ window.AxiomsModule = {

newSelect.innerHTML = '<option value="">Select...</option>';
items.forEach(item => {
newSelect.innerHTML += `<option value="${item.uri || item.name}">${item.name}</option>`;
// See populateSelect() above — prefer the plain name so a stale
// stored `uri` never gets baked into a saved axiom/expression.
newSelect.innerHTML += `<option value="${item.name || item.uri}">${item.name}</option>`;
});

container.appendChild(newSelect);
Expand All @@ -151,7 +161,9 @@ window.AxiomsModule = {

let selectHtml = '<select class="form-select chain-select"><option value="">Select property...</option>';
this.properties.forEach(prop => {
selectHtml += `<option value="${prop.uri || prop.name}">${prop.name}</option>`;
// See populateSelect() above — prefer the plain name so a stale
// stored `uri` never gets baked into a saved axiom/expression.
selectHtml += `<option value="${prop.name || prop.uri}">${prop.name}</option>`;
});
selectHtml += '</select><span class="input-group-text">∘</span>';

Expand Down
30 changes: 21 additions & 9 deletions src/front/static/ontology/js/ontology-swrl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ window.SwrlModule = {
if (typeof d3 !== 'undefined') return Promise.resolve();
return new Promise((resolve, reject) => {
const script = document.createElement('script');
script.src = 'https://d3js.org/d3.v7.min.js';
script.src = 'https://cdn.jsdelivr.net/npm/d3@7/dist/d3.min.js';
script.onload = resolve;
script.onerror = () => reject(new Error('Failed to load D3.js'));
document.head.appendChild(script);
Expand Down Expand Up @@ -415,15 +415,23 @@ window.SwrlModule = {
});

// Only display entities that participate in at least one business
// relationship (object property). Entities with no relationships — or
// with inheritance links only — are hidden, and their inheritance edges
// are dropped so no orphan nodes remain.
// relationship (object property) — plus their direct subclasses, so a
// rule can still reference/classify into a specialized subtype (e.g.
// EstudioIndicadorLEB under EstudioPais) even though the subtype itself
// has no relationships of its own. Entities that are neither connected
// nor a subclass of something connected stay hidden.
const connectedIds = new Set();
links.forEach(l => {
if (l.type !== 'relationship') return;
connectedIds.add(typeof l.source === 'object' ? l.source.id : l.source);
connectedIds.add(typeof l.target === 'object' ? l.target.id : l.target);
});
links.forEach(l => {
if (l.type !== 'inheritance') return;
const parent = typeof l.source === 'object' ? l.source.id : l.source;
const child = typeof l.target === 'object' ? l.target.id : l.target;
if (connectedIds.has(parent)) connectedIds.add(child);
});
nodes = nodes.filter(n => connectedIds.has(n.id));
this._graphNodes = nodes;
links = links.filter(l => {
Expand Down Expand Up @@ -916,11 +924,15 @@ window.SwrlModule = {
}
}

// Raw editor sync
const rawAnt = document.getElementById('swrlRawAntecedent');
const rawCon = document.getElementById('swrlRawConsequent');
if (rawAnt) rawAnt.value = antStr;
if (rawCon) rawCon.value = conStr;
// Raw editor sync — solo si NO estamos en modo raw; en modo raw el texto
// crudo es la fuente de verdad y puede incluir átomos (p.ej. clases THEN
// derivadas) que no existen como nodos del grafo.
if (!this.rawMode) {
const rawAnt = document.getElementById('swrlRawAntecedent');
const rawCon = document.getElementById('swrlRawConsequent');
if (rawAnt) rawAnt.value = antStr;
if (rawCon) rawCon.value = conStr;
}
},

_buildAtomsFromSelection() {
Expand Down
112 changes: 95 additions & 17 deletions src/front/static/query/js/query-cohorts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const CohortModule = {
rules: [], // saved rules list
activeRuleId: null, // id of the loaded saved rule, if any
classes: [], // ontology classes loaded once
baseUri: '', // current ontology base_uri (see _currentUri)
properties: [], // ontology properties loaded once
objectProperties: [], // ObjectProperty subset
lastPreview: null, // last DetectionResult JSON
Expand Down Expand Up @@ -42,17 +43,20 @@ const CohortModule = {
const ont = data?.ontology || data || {};
this.classes = ont.classes || [];
this.properties = ont.properties || [];
this.baseUri = ont.base_uri || ont.baseUri || '';
}
} catch {
this.classes = [];
this.properties = [];
this.baseUri = '';
}
// fall back: read from document if injected
if (!this.classes.length) {
try {
const ont = window.__ontology__ || {};
this.classes = ont.classes || [];
this.properties = ont.properties || [];
this.baseUri = this.baseUri || ont.base_uri || ont.baseUri || '';
} catch { /* noop */ }
}
this.objectProperties = (this.properties || []).filter(p =>
Expand All @@ -61,6 +65,38 @@ const CohortModule = {
this._populateClassSelect();
},

/**
* Reconstruct the CURRENT, correct ontology-form URI for a class or
* property from its local name, using the ``base_uri`` of the
* ontology that's actually loaded right now — instead of trusting
* whatever ``.uri``/``.iri``/``.id`` field the object itself carries.
*
* OntoBricks has repeatedly hit the same namespace-drift bug this
* session (see ``ontology-axioms.js``'s populateSelect/
* addObjectSelect/addChainSelect, and
* ``OntologyGenerator._resolve_uri``): classes/properties can carry
* a stale ``.uri`` minted under a previous namespace (e.g. a
* pre-rebrand ``databricks-ontology.com``) that no longer matches
* the live ``base_uri``. Unlike the OWL generator, nothing on the
* cohort side re-resolves a saved rule's ``class_uri`` / hop
* ``target_class`` against the current base_uri —
* ``CohortBuilder._class_uri_variants`` only tries data-namespace /
* ontology-namespace *rewrites* of whatever URI it's handed, it
* never rebuilds one from a bare name. So a stale ``.uri`` picked
* here gets saved verbatim into the rule and silently breaks class
* membership / target_class matching (0 members, 0 edges) even
* though the entity is otherwise picked correctly. Falls back to
* the object's own uri/iri/id fields only when no base_uri or no
* name is available (e.g. ontology not loaded yet).
*/
_currentUri(item) {
if (!item) return '';
const name = item.name || item.label || '';
const base = (this.baseUri || '').replace(/[#/]+$/, '');
if (base && name) return `${base}#${name}`;
return item.uri || item.iri || item.id || '';
},

_populateClassSelect() {
const sel = document.getElementById('cohortClassUri');
if (!sel) return;
Expand All @@ -69,7 +105,7 @@ const CohortModule = {
(a.label || a.name || a.uri || '').localeCompare(b.label || b.name || b.uri || '')
);
for (const c of classes) {
const uri = c.uri || c.iri || c.id || '';
const uri = this._currentUri(c);
const label = c.label || c.name || uri;
if (!uri) continue;
const opt = document.createElement('option');
Expand All @@ -86,9 +122,11 @@ const CohortModule = {

_classByUri(uri) {
if (!uri) return null;
return (this.classes || []).find(cl =>
(cl.uri || cl.iri || cl.id || '') === uri
) || null;
// Match against the same reconstructed current-namespace URI
// used to populate the select (see _currentUri) — not each
// class's own possibly-stale .uri field — so lookups stay
// consistent with what's actually in <option value>.
return (this.classes || []).find(cl => this._currentUri(cl) === uri) || null;
},

_classNameByUri(uri) {
Expand All @@ -111,13 +149,31 @@ const CohortModule = {
},

_dataPropsForClass(classUri) {
const clsName = this._classNameByUri(classUri || '');
const dataProps = (this.properties || []).filter(p =>
!(p.type || p.kind || '').toLowerCase().includes('object')
);
if (!clsName) return dataProps;
const matched = dataProps.filter(p => (p.domain || '') === clsName);
return matched.length ? matched : dataProps;
const cls = this._classByUri(classUri || '');
if (!cls) return [];
const raw = cls.dataProperties || cls.properties || cls.attributes || [];
// Always rebuild the URI from the currently-loaded base_uri (see
// _currentUri) rather than trust each property's own stored
// uri/iri/id — those can carry a stale pre-rebrand namespace
// (same bug class fixed in ontology-axioms.js) and CohortBuilder
// has no fallback to re-resolve a foreign-namespace data-property
// URI against attribute values, so a stale URI here silently
// breaks "where"/"conditions" filters. Some ontology payloads
// also only send {name, localName} with no uri/iri/id at all —
// _currentUri's name-based reconstruction covers that case too,
// so <option value> is never empty (which used to make the
// selected property silently fail to persist, dropped by
// _sanitizedRulePayload's `w.property` check on save).
const domainRoot = (classUri || '').split('#')[0];
return raw
.filter(p => !(p.type || p.kind || '').toLowerCase().includes('object'))
.map(p => {
const local = p.localName || p.name || '';
const uri = this._currentUri(p)
|| (p.uri || p.iri || p.id)
|| (local ? `${domainRoot}#${local}` : '');
return uri ? Object.assign({}, p, { uri }) : p;
});
},

_compatibleViaProperties(sourceUri, targetUri) {
Expand All @@ -132,7 +188,7 @@ const CohortModule = {
_compatibleTargetClasses(sourceUri, viaUri) {
const props = this._objectPropsForSource(sourceUri);
const filtered = viaUri
? props.filter(p => (p.uri || p.iri || p.id || '') === viaUri)
? props.filter(p => this._currentUri(p) === viaUri)
: props;
const ranges = new Set();
for (const p of filtered) {
Expand Down Expand Up @@ -234,6 +290,7 @@ const CohortModule = {
this._hydrateForm();
this._renderRulesList();
this._showBuildTab();
this._syncDeleteBtn();
this._setStatus(`Loaded rule "${r.label || r.id}"`);
},

Expand All @@ -258,9 +315,30 @@ const CohortModule = {
if (reset) this._hydrateForm();
this._renderRulesList();
this._renderRuleSummary();
this._syncDeleteBtn();
this._setStatus('Drafting a new rule.');
},

// ---- Delete (toolbar button, mirrors New rule / Save rule) ---------
//
// ``deleteRule(id)`` already existed (DELETE /dtwin/cohorts/rules/:id)
// but was only reachable indirectly; nothing in the design-page
// toolbar called it. This adds a direct "Delete rule" action next to
// Save rule, gated to the currently loaded/saved rule so there's
// never any ambiguity about what gets deleted.
_syncDeleteBtn() {
const btn = document.getElementById('cohortDeleteBtn');
if (btn) btn.disabled = !this.activeRuleId;
},

deleteActiveRule() {
if (!this.activeRuleId) {
this._notify('No rule loaded to delete — pick one from the list first.', 'warning');
return;
}
this.deleteRule(this.activeRuleId);
},

_resetPreviewPane() {
const body = document.getElementById('cohortPreviewBody');
if (body) {
Expand Down Expand Up @@ -750,12 +828,12 @@ const CohortModule = {
_renderHopRow(linkIdx, hopIdx, hop, sourceUri, isTerminal) {
const viaProps = this._compatibleViaProperties(sourceUri, hop.target_class);
if (hop.via && !viaProps.some(p =>
(p.uri || p.iri || p.id || '') === hop.via)) {
this._currentUri(p) === hop.via)) {
hop.via = '';
}
const targetClasses = this._compatibleTargetClasses(sourceUri, hop.via);
if (hop.target_class && !targetClasses.some(c =>
(c.uri || c.iri || c.id || '') === hop.target_class)) {
this._currentUri(c) === hop.target_class)) {
hop.target_class = '';
}
const viaDisabled = !sourceUri;
Expand All @@ -776,7 +854,7 @@ const CohortModule = {
? 'pick previous entity first'
: (viaProps.length ? '— predicate —' : 'no compatible relationship')}</option>
${viaProps.map(p => {
const uri = p.uri || p.iri || p.id || '';
const uri = this._currentUri(p);
const lbl = p.label || p.name || uri;
return `<option value="${this._esc(uri)}" ${uri === hop.via ? 'selected' : ''}>${this._esc(lbl)}</option>`;
}).join('')}
Expand All @@ -794,7 +872,7 @@ const CohortModule = {
? 'pick predicate first'
: (targetClasses.length ? '— entity —' : 'no compatible target')}</option>
${targetClasses.map(c => {
const uri = c.uri || c.iri || c.id || '';
const uri = this._currentUri(c);
const lbl = c.label || c.name || uri;
return `<option value="${this._esc(uri)}" ${uri === hop.target_class ? 'selected' : ''}>${this._esc(lbl)}</option>`;
}).join('')}
Expand All @@ -816,7 +894,7 @@ const CohortModule = {
wrap.querySelector('.cohort-hop-via').onchange = (e) => {
hop.via = e.target.value;
const stillValid = this._compatibleTargetClasses(sourceUri, hop.via)
.some(c => (c.uri || c.iri || c.id || '') === hop.target_class);
.some(c => this._currentUri(c) === hop.target_class);
if (!stillValid) hop.target_class = '';
this._renderLinks();
this.markDirty();
Expand Down
5 changes: 5 additions & 0 deletions src/front/templates/partials/ontology/_ontology_cohorts.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ <h4 class="mb-1"><i class="bi bi-people-fill me-2"></i>Cohorts</h4>
onclick="CohortModule.save()">
<i class="bi bi-check2 me-1"></i>Save rule
</button>
<button class="btn btn-sm btn-outline-danger" id="cohortDeleteBtn"
onclick="CohortModule.deleteActiveRule()" disabled
title="Delete the currently loaded rule">
<i class="bi bi-trash me-1"></i>Delete rule
</button>
<button type="button" class="btn btn-sm btn-outline-primary onto-discuss-btn"
title="Open the ontology discussion" onclick="openOntologyDiscussion()">
<i class="bi bi-chat-dots"></i>
Expand Down