Skip to content

Commit 917a3a7

Browse files
Jurij Skornikclaude
andcommitted
fix(node-ui): round-3 Codex — SWM assertionGraph guard + VM/discarded empty-state copy
Round-3 review fixes on the S4 assertion detail view (kept as a focused 4th commit rather than folded into C1 — the fixup's helpers.ts hunk overlaps C2's breadcrumb helpers + C3's primarySubGraphOf, so an autosquash into C1 conflicts mechanically; a standalone commit is the clean, conflict-free shape team-lead OK'd). - Finding 2 (api.ts fetchAssertionState): the assertionGraph fallback was URI-shape-unsafe. For an SWM input (a `urn:dkg:assertion:…` lifecycle URN) whose `dkg:assertionGraph` did NOT resolve (legacy/partial `_meta` row), echoing the URN made fetchAssertionTriples query `GRAPH <urn:…>` (a graph that never holds triples) → bogus render. Now: urn-input + unresolved → assertionGraph: undefined (Triples/Entities fall to their empty-state). WM data-graph-URI input still echoes itself. - Finding 3 (assertionEmptyStateCopy, ux §4.7.1 locked): the empty-state copy now keys off `dkg:state` (4 branches) instead of special-casing promoted only — published/finalized show the VM / Knowledge-Assets line ("entities → Knowledge Assets" at the VM boundary, §4.8), discarded is terminal, created/promoted unchanged. Plain text, no links. - Finding 1 (remote SWM state) reply-not-valid (no replicated state source); finding-1 round-2 discarded-neutral badge + finding-2 round-2 literal-display decode already shipped earlier in C1. Tests: SWM-legacy-no-assertionGraph guard + WM-echo counterpart (use-assertion-state); assertionEmptyStateCopy 4-branch truth table (assertion-detail-helpers); published empty-state DOM render (assertion-detail-view). Finding-2 guard negative-proofed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9f3025c commit 917a3a7

10 files changed

Lines changed: 1131 additions & 142 deletions

packages/node-ui/src/ui/api.ts

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -841,10 +841,22 @@ export async function fetchAssertionState(
841841
const metaGraph = `did:dkg:context-graph:${contextGraphId}/_meta`;
842842
// UNION admits both `graphUri` shapes (WM data-graph URI via the
843843
// inverse `dkg:assertionGraph` link; SWM lifecycle URN directly).
844+
//
845+
// Codex round-4 — branch A binds the input AS `?lifecycle`
846+
// UNCONDITIONALLY and resolves `dkg:assertionGraph` only OPTIONALLY, so
847+
// a legacy/partial SWM row whose lifecycle URN carries `dkg:state` but
848+
// NOT `dkg:assertionGraph` still resolves its state (pre-fix, branch A
849+
// required the assertionGraph triple to bind `?lifecycle`, so such a
850+
// row read no state → "state unavailable" though the state exists).
851+
// The outer `?lifecycle dkg:state` then gates the row: for a WM
852+
// data-graph-URI input, branch A binds `?lifecycle = <data-graph URI>`
853+
// but `<data-graph URI> dkg:state` never matches (state lives on the
854+
// lifecycle URN), so branch A contributes nothing and branch B handles
855+
// WM exactly as before. So WM→B, SWM→A, SWM-no-assertionGraph→A.
844856
const sparql = `SELECT ?state ?layer ?createdBy ?assertionGraph WHERE {
845857
GRAPH <${metaGraph}> {
846-
{ <${graphUri}> <${DKG}assertionGraph> ?assertionGraph .
847-
BIND(<${graphUri}> AS ?lifecycle) }
858+
{ BIND(<${graphUri}> AS ?lifecycle)
859+
OPTIONAL { <${graphUri}> <${DKG}assertionGraph> ?assertionGraph } }
848860
UNION
849861
{ ?lifecycle <${DKG}assertionGraph> <${graphUri}> .
850862
BIND(<${graphUri}> AS ?assertionGraph) }
@@ -871,13 +883,23 @@ export async function fetchAssertionState(
871883
state === 'created' ? 'wm' :
872884
state === 'promoted' ? 'swm' :
873885
'vm';
886+
// The data graph to read triples from. Prefer the resolved
887+
// `dkg:assertionGraph`. Codex round-3 — the fallback to `graphUri`
888+
// itself is only safe when the INPUT is already a data-graph URI (the
889+
// WM shape). For an SWM input (a `urn:dkg:assertion:…` lifecycle URN)
890+
// whose `dkg:assertionGraph` did NOT resolve (legacy/partial `_meta`
891+
// row), echoing the URN would make `fetchAssertionTriples` query
892+
// `GRAPH <urn:dkg:assertion:…>` — a graph that never holds triples —
893+
// and render a bogus "data" set. In that case return `undefined` so
894+
// the Triples/Entities panes fall to their empty-state instead.
895+
const resolvedAssertionGraph = bv(first.assertionGraph);
896+
const inputIsLifecycleUrn = graphUri.startsWith('urn:dkg:assertion:');
897+
const assertionGraph = resolvedAssertionGraph
898+
?? (inputIsLifecycleUrn ? undefined : graphUri);
874899
return {
875900
state,
876901
layer,
877-
// The data graph to read triples from: the resolved
878-
// `dkg:assertionGraph` (correct for the SWM lifecycle-URN input),
879-
// falling back to the input itself (the WM data-graph URI input).
880-
assertionGraph: bv(first.assertionGraph) ?? graphUri,
902+
assertionGraph,
881903
createdBy: bv(first.createdBy),
882904
};
883905
}
@@ -945,14 +967,38 @@ export async function fetchAssertionTriples(
945967
* N-Triples string form is already fully encoded by the daemon, so it
946968
* passes through verbatim.
947969
*/
970+
/**
971+
* Escape a literal body for the N-Triples `"…"` form: backslash + quote,
972+
* the canonical short escapes for tab/newline/carriage-return, and any
973+
* other C0 control char as `\\uXXXX`. Order matters — backslash first so
974+
* the escapes we add aren't re-escaped.
975+
*/
976+
function escapeNTriplesLiteral(value: string): string {
977+
let out = "";
978+
for (const ch of value) {
979+
const code = ch.codePointAt(0)!;
980+
if (ch === '\\') out += '\\\\';
981+
else if (ch === '"') out += '\\"';
982+
else if (code === 0x09) out += '\\t';
983+
else if (code === 0x0a) out += '\\n';
984+
else if (code === 0x0d) out += '\\r';
985+
else if (code < 0x20) out += '\\u' + code.toString(16).padStart(4, '0').toUpperCase();
986+
else out += ch;
987+
}
988+
return out;
989+
}
990+
948991
function rawBindingValue(v: unknown): string | undefined {
949992
if (v == null) return undefined;
950993
if (typeof v === 'object' && 'value' in (v as any)) {
951994
const node = v as any;
952995
if (node.type === 'literal' || node.type === 'typed-literal') {
953-
// Escape per N-Triples so embedded quotes/backslashes don't break
954-
// the leading-`"` classification or the renderers.
955-
const escaped = String(node.value).replace(/\\/g, '\\\\').replace(/"/g, '\\"');
996+
// Escape per N-Triples so embedded quotes/backslashes/control chars
997+
// don't break the leading-`"` classification or the renderers.
998+
// Codex round-6 — control chars (raw \n/\r/\t in a multiline
999+
// extracted value, plus other C0 controls) were previously left
1000+
// raw → invalid N-Triples → broke the graph/triple parsers.
1001+
const escaped = escapeNTriplesLiteral(String(node.value));
9561002
// Standard SPARQL JSON carries the datatype on `.datatype` and the
9571003
// language on `.xml:lang` (some serialisers use `.language`).
9581004
const lang = node['xml:lang'] ?? node.language;
@@ -961,6 +1007,12 @@ function rawBindingValue(v: unknown): string | undefined {
9611007
if (datatype) return `"${escaped}"^^<${datatype}>`;
9621008
return `"${escaped}"`;
9631009
}
1010+
// Codex round-5 — a blank node must come back as `_:<id>`, not the
1011+
// bare identifier; otherwise downstream (buildMemoryEntities + the
1012+
// graph) misclassify it (a bare id is neither a leading-`"` literal
1013+
// nor a recognisable resource → bnode RDF structure disappears /
1014+
// mislabels). Branch BEFORE the generic fallthrough.
1015+
if (node.type === 'bnode') return `_:${String(node.value)}`;
9641016
return String(node.value);
9651017
}
9661018
if (typeof v === 'string') return v;

packages/node-ui/src/ui/hooks/useMemoryEntities.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { useState, useEffect, useMemo, useRef, useCallback } from 'react';
22
import { postQueryDeduped } from '../api.js';
33
import { useMemoryGraphEvents } from './useNodeEvents.js';
44
import { MEMORY_LABEL_PREDICATES } from '../lib/memoryLabels.js';
5+
import { decodeRdfStringLiteral } from '../../rdf-literal.js';
56

67
export type TrustLevel = 'working' | 'shared' | 'verified';
78
export type MemoryLayerKey = 'wm' | 'swm' | 'vm';
@@ -107,7 +108,11 @@ export function canonicalEntityUri(uri: string): string {
107108

108109
function shortLabel(uri: string): string {
109110
if (!uri) return '—';
110-
if (uri.startsWith('"')) return uri.replace(/^"|"$/g, '');
111+
// Codex round-6 — decode RDF literals (drop the `^^<type>`/`@lang`
112+
// suffix + unescape the body) instead of a bare outer-quote strip, so
113+
// `"Hola"@es` renders `Hola`, not `Hola"@es`. Idempotent for a
114+
// suffix-free `"Hola"` and a no-op for non-literal IRIs.
115+
if (uri.startsWith('"')) return decodeRdfStringLiteral(uri);
111116
const hash = uri.lastIndexOf('#');
112117
const slash = uri.lastIndexOf('/');
113118
const cut = Math.max(hash, slash);
@@ -487,7 +492,11 @@ export function buildEntities(layered: LayeredTriple[]): Map<string, MemoryEntit
487492
}
488493
} else {
489494
const existing = entity.properties.get(t.predicate) ?? [];
490-
const val = t.object.startsWith('"') ? t.object.replace(/^"|"$/g, '') : t.object;
495+
// Codex round-6 — decode the literal (drop datatype/lang suffix +
496+
// unescape) rather than a bare outer-quote strip, so property
497+
// values used as labels (deriveEntityLabel) render `Hola`, not
498+
// `Hola"@es`. Idempotent for suffix-free / plain values.
499+
const val = t.object.startsWith('"') ? decodeRdfStringLiteral(t.object) : t.object;
491500
if (!existing.includes(val)) {
492501
existing.push(val);
493502
entity.properties.set(t.predicate, existing);

packages/node-ui/src/ui/views/ProjectView.tsx

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -330,13 +330,21 @@ export function ProjectView({ contextGraphId }: ProjectViewProps) {
330330
setSelectedAssertion(assertion);
331331
}, [captureDetailOrigin]);
332332

333+
// Consumes a pending scroll restore once the detail overlay has
334+
// actually closed. S4 — the close path is shared by the entity AND
335+
// the assertion overlay (`handleDetailClose` queues `origin.scroll`
336+
// for both), so the guard must wait for BOTH to clear and the deps
337+
// must include `selectedAssertion`. Without it, closing an assertion
338+
// back to the same layer it opened from changes none of the other
339+
// listed deps, the effect never re-fires, and the queued scroll is
340+
// never restored — an M2-parity gap vs the entity-close path (7-1).
333341
useEffect(() => {
334-
if (selectedUri) return;
342+
if (selectedUri || selectedAssertion) return;
335343
const scroll = pendingScrollRestoreRef.current;
336344
if (!scroll) return;
337345
pendingScrollRestoreRef.current = null;
338346
restoreScroll(scroll);
339-
}, [selectedUri, activeLayer, activeSubGraph, layerContentTabs, subGraphTabs, restoreScroll]);
347+
}, [selectedUri, selectedAssertion, activeLayer, activeSubGraph, layerContentTabs, subGraphTabs, restoreScroll]);
340348

341349
// Cross-tab entity open — e.g. the agent profile page in another tab
342350
// fires a CustomEvent("v10:open-entity", { contextGraphId, entityUri })
@@ -696,11 +704,20 @@ export function ProjectView({ contextGraphId }: ProjectViewProps) {
696704
setSelectedLayerContext(prev => layerContext ?? (hadSelection ? prev : null));
697705
const currentSubGraph = activeSubGraphRef.current;
698706
if (currentSubGraph) {
699-
const targetSubGraph = primarySubGraphOf(
700-
entitiesRef.current.get(uri) ?? entitiesRef.current.get(canonicalEntityUri(uri)),
701-
);
702-
if (targetSubGraph && targetSubGraph !== currentSubGraph) {
703-
setActiveSubGraphSync(targetSubGraph);
707+
const entity = entitiesRef.current.get(uri) ?? entitiesRef.current.get(canonicalEntityUri(uri));
708+
// Codex round-8 (8-3) — prefer the CURRENT subgraph for a multi-homed
709+
// entity. M2(b) switches to `primarySubGraphOf` (first non-meta),
710+
// which is arbitrary when the entity lives in several subgraphs. If
711+
// the entity is ALREADY in the current subgraph, stay — don't jump to
712+
// an arbitrary sibling. Only switch when the entity isn't in the
713+
// current subgraph at all; then `primarySubGraphOf` is the must-move
714+
// fallback (M2(b)'s reason for existing: un-strand an entity that
715+
// doesn't belong to the page you're on). `subGraphs` is a Set<string>.
716+
if (!entity?.subGraphs.has(currentSubGraph)) {
717+
const targetSubGraph = primarySubGraphOf(entity);
718+
if (targetSubGraph && targetSubGraph !== currentSubGraph) {
719+
setActiveSubGraphSync(targetSubGraph);
720+
}
704721
}
705722
}
706723
}, [openEntityDetail, setActiveSubGraphSync]);
@@ -788,6 +805,29 @@ export function ProjectView({ contextGraphId }: ProjectViewProps) {
788805
? selectedAssertion.name
789806
: null;
790807

808+
// Codex round-8 (8-2) — when a detail is open the breadcrumb's middle
809+
// hop must label the M2 ORIGIN (where the middle-hop click returns you),
810+
// not the current location. After an M2(b) cross-subgraph follow the
811+
// active subgraph diverges from the origin, so the current-scope label
812+
// would name the followed-into subgraph while clicking returns to the
813+
// origin. Read the origin snapshot (captured at detail-open) and resolve
814+
// its subgraph display name via the profile. Only meaningful when a
815+
// detail is open — `breadcrumbDetailLabel` gates the read, and the
816+
// detail-open selection state drives the render, so the ref is populated
817+
// by the time we read it here.
818+
const breadcrumbOrigin = breadcrumbDetailLabel ? detailOriginRef.current : null;
819+
const breadcrumbOriginSubGraph = breadcrumbOrigin?.activeSubGraph ?? null;
820+
// Codex round-10 (10-1) — `forSubGraph` returns undefined for a subgraph
821+
// with NO profile binding, so `.displayName` would throw before the
822+
// breadcrumb renders (opening a detail from an unprofiled subgraph would
823+
// crash). Optional-chain and degrade to the slug — the SAME fallback the
824+
// no-origin-subgraph case uses. (The 9-2 chrome accesses in
825+
// ProjectHeaderStrip already optional-chain `forSubGraph`; this is the one
826+
// un-guarded access, which resolves only the breadcrumb label.)
827+
const breadcrumbOriginSubGraphName = breadcrumbOriginSubGraph
828+
? profile.forSubGraph(breadcrumbOriginSubGraph)?.displayName ?? breadcrumbOriginSubGraph
829+
: null;
830+
791831
// Codex Bug C — gate the `entities`-driven chip-count path on a
792832
// fully-loaded memory snapshot. While `useMemoryEntities` is mid-
793833
// hydration or a layer query is in-flight, `entityList` is partial
@@ -828,6 +868,9 @@ export function ProjectView({ contextGraphId }: ProjectViewProps) {
828868
activeLayer={activeLayer}
829869
activeSubGraph={activeSubGraphBinding}
830870
detailLabel={breadcrumbDetailLabel}
871+
originLayer={breadcrumbOrigin?.activeLayer ?? null}
872+
originSubGraph={breadcrumbOriginSubGraph}
873+
originSubGraphDisplayName={breadcrumbOriginSubGraphName}
831874
onOverview={handleBreadcrumbOverview}
832875
onRestoreOrigin={handleDetailClose}
833876
/>
@@ -865,7 +908,12 @@ export function ProjectView({ contextGraphId }: ProjectViewProps) {
865908
<AssertionDetailView
866909
assertion={selectedAssertion}
867910
contextGraphId={contextGraphId}
868-
onNavigate={handleNavigate}
911+
// Codex round-5 — forward the assertion's layer into
912+
// handleNavigate's existing `layerContext` (3rd) param so the
913+
// follow-on entity detail stays scoped to the assertion's layer.
914+
// AssertionDetailView passes `layer` as the 2nd arg; map it to
915+
// the 3rd param (originScrollKey stays undefined).
916+
onNavigate={(uri, layer) => handleNavigate(uri, undefined, layer)}
869917
onComplete={rawMemory.refresh}
870918
onOpenAgent={openAgent}
871919
/>

0 commit comments

Comments
 (0)