From a914313e3a39eaa9f7196cce8bdab11326cc1725 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Sat, 20 Jun 2026 16:34:32 -0700 Subject: [PATCH] =?UTF-8?q?perf(8.0):=20visibility-aware=20fast=20adjacenc?= =?UTF-8?q?y=20=E2=80=94=20related()=20stays=20O(degree)=20under=20default?= =?UTF-8?q?=20visibility?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit related({from/to}) routes through storage.getVerbs, which has O(degree) GraphAdjacencyIndex fast paths (getVerbsBySource/Target/Type_internal). But those fast paths were disqualified whenever excludeVisibility was set — and every default related() sets it, because visibility defaults to excluding internal+system. So the common per-node edge lookup fell through to a full O(E) shard scan (multi-second per node on large graphs; a consumer measured 1.4-4.6s/node and an O(N^2) whole-graph walk). Root cause two-parter, both fixed here: - hydrateVerbWithMetadata dropped 'visibility' (mapped subtype but not the reserved visibility field), so fast-path results couldn't be visibility-filtered AND related({includeInternal}) couldn't even report which edges were internal (latent correctness bug — verbsToRelations already mapped it). Now hydrated. - getVerbs disqualified the fast paths on subtype/excludeVisibility. Now the fast paths apply both filters on their already-hydrated O(degree) candidate set via applyVerbMetadataFilters() — matching the full-scan fallback's semantics — so default related() stays on the index instead of scanning the whole graph. Filtering semantics are unchanged (same result set as the scan); only the path that computes it changes. Test: related-visibility-fast-path.test.ts (default excludes internal both directions, includeInternal surfaces + reports visibility, subtype filter on the fast path). --- src/storage/baseStorage.ts | 105 ++++++++++++++---- .../related-visibility-fast-path.test.ts | 81 ++++++++++++++ 2 files changed, 165 insertions(+), 21 deletions(-) create mode 100644 tests/unit/brainy/related-visibility-fast-path.test.ts diff --git a/src/storage/baseStorage.ts b/src/storage/baseStorage.ts index 88fb17e6..d8e90e0f 100644 --- a/src/storage/baseStorage.ts +++ b/src/storage/baseStorage.ts @@ -1119,6 +1119,12 @@ export abstract class BaseStorage extends BaseStorageAdapter { ...verb, // Standard fields at top-level subtype: reserved.subtype as string | undefined, + // visibility is a reserved top-level field (the verb mirror of Entity.visibility). + // Surfacing it here lets the graph-index fast paths apply visibility filtering on + // their already-hydrated results (so default related() stays O(degree) instead of + // falling through to a full scan), and lets related({ includeInternal }) results + // actually report which edges are internal. + visibility: reserved.visibility as HNSWVerbWithMetadata['visibility'], createdAt: normalizeStoredTimestamp(reserved.createdAt), updatedAt: normalizeStoredTimestamp(reserved.updatedAt), confidence: reserved.confidence as number | undefined, @@ -1131,6 +1137,45 @@ export abstract class BaseStorage extends BaseStorageAdapter { } } + /** + * @description Apply the metadata-derived verb filters (`subtype`, + * `excludeVisibility`) that the graph-index fast paths can satisfy on their + * already-hydrated O(degree) / O(type) candidate set — matching the semantics + * of the full-scan fallback in {@link getVerbsWithPagination}. This is what + * keeps default `related({ from/to })` (which always sets the visibility + * exclusion) on the fast adjacency path instead of forcing a full O(E) scan. + * + * - `subtype`: keeps only verbs carrying a matching subtype; a verb with no + * subtype is excluded when a subtype filter is set (same as the fallback). + * - `excludeVisibility`: drops verbs whose stored visibility tier is in the + * excluded set; an absent tier is `'public'` and is always kept. + * + * @param verbs - Hydrated candidate verbs from a fast-path lookup. + * @param filter - The `getVerbs` filter (only `subtype` / `excludeVisibility` + * are read here; the structural match was already done by the caller). + * @returns The candidates with the metadata filters applied (input order preserved). + */ + protected applyVerbMetadataFilters( + verbs: HNSWVerbWithMetadata[], + filter?: { + subtype?: string | string[] + excludeVisibility?: string[] + } + ): HNSWVerbWithMetadata[] { + let out = verbs + const subtypeFilter = filter?.subtype + if (subtypeFilter) { + const allowed = new Set(Array.isArray(subtypeFilter) ? subtypeFilter : [subtypeFilter]) + out = out.filter((v) => v.subtype !== undefined && allowed.has(v.subtype)) + } + const excludeVisibility = filter?.excludeVisibility + if (excludeVisibility && excludeVisibility.length > 0) { + const excluded = new Set(excludeVisibility) + out = out.filter((v) => !(v.visibility && excluded.has(v.visibility))) + } + return out + } + /** * Get a noun from storage (returns combined HNSWNounWithMetadata) * @param id Entity ID @@ -1851,17 +1896,15 @@ export abstract class BaseStorage extends BaseStorageAdapter { const cursor = pagination.cursor // Optimize for common filter cases to avoid loading all verbs. - // NOTE: subtype filter intentionally disqualifies the fast paths and forces - // fallthrough to the full shard scan (which loads metadata and applies the - // subtype check after the load). Subtype is not stored on the raw HNSWVerb; - // it's a metadata field. The graph-index fast paths return verbs without - // metadata, so they can't apply subtype filtering correctly. The 8.0 - // `excludeVisibility` filter (also a metadata field) disqualifies them the same way. - if ( - options?.filter && - !(options.filter as { verbType?: string | string[]; subtype?: string | string[] }).subtype && - !(options.filter as { excludeVisibility?: string[] }).excludeVisibility - ) { + // The graph-index fast paths (getVerbsBySource/Target/Type_internal) hydrate + // each candidate's metadata, so `subtype` and the 8.0 `excludeVisibility` + // filters — both metadata fields — are applied on the small O(degree) / + // O(type) candidate set via applyVerbMetadataFilters() BEFORE pagination. + // (They used to disqualify the fast paths and force a full O(E) shard scan, + // which made default related({ from/to }) — which always sets the visibility + // exclusion — scan the entire graph per node.) An arbitrary `metadata` filter + // still falls through, since each block guards `!options.filter.metadata`. + if (options?.filter) { // CRITICAL VFS FIX: If filtering by sourceId + verbType (most common VFS pattern!) // This is the query PathResolver.getChildren() uses: related({ from: dirId, type: VerbType.Contains }) if ( @@ -1879,9 +1922,13 @@ export abstract class BaseStorage extends BaseStorageAdapter { ? options.filter.verbType[0] : options.filter.verbType - // Get verbs by source, then filter by type (O(1) graph lookup + O(n) type filter) + // Get verbs by source, then filter by type (O(1) graph lookup + O(n) type filter), + // then apply the subtype / visibility metadata filters on the candidate set. const verbsBySource = await this.getVerbsBySource_internal(sourceId) - const filteredVerbs = verbsBySource.filter(v => v.verb === verbType) + const filteredVerbs = this.applyVerbMetadataFilters( + verbsBySource.filter(v => v.verb === verbType), + options.filter + ) // Apply pagination const paginatedVerbs = filteredVerbs.slice(offset, offset + limit) @@ -1914,8 +1961,12 @@ export abstract class BaseStorage extends BaseStorageAdapter { ? options.filter.sourceId[0] : options.filter.sourceId - // Get verbs by source directly - const verbsBySource = await this.getVerbsBySource_internal(sourceId) + // Get verbs by source directly (hydrated with metadata), then apply the + // subtype / visibility metadata filters on the O(degree) candidate set. + const verbsBySource = this.applyVerbMetadataFilters( + await this.getVerbsBySource_internal(sourceId), + options.filter + ) // Apply pagination const paginatedVerbs = verbsBySource.slice(offset, offset + limit) @@ -1948,8 +1999,12 @@ export abstract class BaseStorage extends BaseStorageAdapter { ? options.filter.targetId[0] : options.filter.targetId - // Get verbs by target directly - const verbsByTarget = await this.getVerbsByTarget_internal(targetId) + // Get verbs by target directly (hydrated with metadata), then apply the + // subtype / visibility metadata filters on the O(degree) candidate set. + const verbsByTarget = this.applyVerbMetadataFilters( + await this.getVerbsByTarget_internal(targetId), + options.filter + ) // Apply pagination const paginatedVerbs = verbsByTarget.slice(offset, offset + limit) @@ -1982,8 +2037,12 @@ export abstract class BaseStorage extends BaseStorageAdapter { ? options.filter.verbType[0] : options.filter.verbType - // Get verbs by type directly - const verbsByType = await this.getVerbsByType_internal(verbType) + // Get verbs by type directly (hydrated with metadata), then apply the + // subtype / visibility metadata filters on the candidate set. + const verbsByType = this.applyVerbMetadataFilters( + await this.getVerbsByType_internal(verbType), + options.filter + ) // Apply pagination const paginatedVerbs = verbsByType.slice(offset, offset + limit) @@ -2026,8 +2085,12 @@ export abstract class BaseStorage extends BaseStorageAdapter { // Get verbs by source (uses GraphAdjacencyIndex if available) const verbsBySource = await this.getVerbsBySource_internal(sourceId) - // Filter by verbType in memory (fast - usually small number of verbs per source) - const filtered = verbsBySource.filter(v => verbTypes.includes(v.verb)) + // Filter by verbType in memory (fast - usually small number of verbs per source), + // then apply the subtype / visibility metadata filters on the candidate set. + const filtered = this.applyVerbMetadataFilters( + verbsBySource.filter(v => verbTypes.includes(v.verb)), + options.filter + ) // Apply pagination const paginatedVerbs = filtered.slice(offset, offset + limit) diff --git a/tests/unit/brainy/related-visibility-fast-path.test.ts b/tests/unit/brainy/related-visibility-fast-path.test.ts new file mode 100644 index 00000000..7b289390 --- /dev/null +++ b/tests/unit/brainy/related-visibility-fast-path.test.ts @@ -0,0 +1,81 @@ +/** + * @module tests/unit/brainy/related-visibility-fast-path + * @description Graph-perf #1 (8.0): the visibility-aware fast adjacency path. + * + * `related({ from })` / `related({ to })` route through the O(degree) + * GraphAdjacencyIndex fast paths in `storage.getVerbs`. Before this fix, the + * default visibility exclusion (`excludeVisibility: ['internal','system']`, + * which every default `related()` sets) DISQUALIFIED those fast paths and forced + * a full O(E) shard scan — the root cause of multi-second per-node edge lookups + * on large graphs. The fast paths now hydrate each candidate's metadata and + * apply the subtype / visibility filters on the small O(degree) candidate set. + * + * Two observable contracts are pinned here (the perf change itself is structural + * — these prove the fast path produces the SAME correct results the scan did): + * 1. default `related()` excludes `internal` / `system` edges; `includeInternal` + * / `includeSystem` opt them back in. + * 2. returned `Relation`s now carry `visibility` (previously dropped by + * `hydrateVerbWithMetadata`, so `related({ includeInternal })` could not even + * report which edges were internal). + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { Brainy } from '../../../src/index.js' +import { NounType, VerbType } from '../../../src/types/graphTypes.js' +import { createTestConfig } from '../../helpers/test-factory.js' + +describe('related() visibility-aware fast adjacency (graph-perf #1)', () => { + let brain: Brainy + let a: string, b: string, c: string, d: string + + beforeEach(async () => { + brain = new Brainy(createTestConfig()) + await brain.init() + a = await brain.add({ type: NounType.Person, subtype: 'employee', data: 'A' }) + b = await brain.add({ type: NounType.Person, subtype: 'employee', data: 'B' }) + c = await brain.add({ type: NounType.Person, subtype: 'employee', data: 'C' }) + d = await brain.add({ type: NounType.Person, subtype: 'employee', data: 'D' }) + // A → B public, A → C internal, A → D public. + await brain.relate({ from: a, to: b, type: VerbType.RelatedTo, subtype: 'colleague' }) + await brain.relate({ from: a, to: c, type: VerbType.RelatedTo, subtype: 'colleague', visibility: 'internal' }) + await brain.relate({ from: a, to: d, type: VerbType.RelatedTo, subtype: 'colleague' }) + }) + + afterEach(async () => { + await brain.close() + }) + + it('default related({ from }) excludes internal edges (out-direction fast path)', async () => { + const edges = await brain.related({ from: a }) + const targets = edges.map((e) => e.to).sort() + expect(targets).toEqual([b, d].sort()) + // None of the default results carry a non-public visibility. + expect(edges.every((e) => e.visibility === undefined || e.visibility === 'public')).toBe(true) + }) + + it('related({ from, includeInternal }) surfaces the internal edge AND reports its visibility', async () => { + const edges = await brain.related({ from: a, includeInternal: true }) + expect(edges.map((e) => e.to).sort()).toEqual([b, c, d].sort()) + const internalEdge = edges.find((e) => e.to === c) + // The latent-bug fix: visibility now flows through hydration to the Relation. + expect(internalEdge?.visibility).toBe('internal') + }) + + it('default related({ to }) excludes an internal edge (in-direction fast path)', async () => { + // B has one public in-edge; C has one internal in-edge. + const intoB = await brain.related({ to: b }) + expect(intoB.map((e) => e.from)).toEqual([a]) + + const intoC = await brain.related({ to: c }) + expect(intoC).toHaveLength(0) // internal edge hidden by default + + const intoCAll = await brain.related({ to: c, includeInternal: true }) + expect(intoCAll.map((e) => e.from)).toEqual([a]) + expect(intoCAll[0]?.visibility).toBe('internal') + }) + + it('subtype filter still works on the fast path (returns only matching, excludes internal)', async () => { + const colleagues = await brain.related({ from: a, subtype: 'colleague' }) + expect(colleagues.map((e) => e.to).sort()).toEqual([b, d].sort()) + }) +})