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()) + }) +})