From 21d02d3baecc44733b18c9201e04eb3da414603a Mon Sep 17 00:00:00 2001 From: David Snelling Date: Wed, 17 Jun 2026 17:07:58 -0700 Subject: [PATCH] fix(8.0): stats() per-type counts no longer inflate with HNSW re-saves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nounCountsByType (read by stats().entitiesByType / counts.byTypeEnum) was incremented in saveNoun_internal(), which runs on every noun write — including the HNSW index re-saving a node whenever its neighbor links change. So per-type counts grew with graph connectivity instead of entity count (an internal report saw 8 documents read as 44). Moved the increment into saveNounMetadata_internal(), gated on isNew + visibility, exactly parallel to the authoritative total; the visibility-flip path adjusts it in lockstep too. Tests: tests/unit/storage/stats-count-accuracy.test.ts (30 dense-type entities -> exactly 30 across stats/counts/getNounCount) + de-theatricalized the >=2 assertion in brainy-core.unit.test.ts to exact equality. Build green; 1455 unit green. Part of the 8.0 readiness audit Phase 1; cold-reopen count rehydration is still WIP. --- src/storage/baseStorage.ts | 27 +++++---- tests/unit/brainy-core.unit.test.ts | 6 +- .../unit/storage/stats-count-accuracy.test.ts | 58 +++++++++++++++++++ 3 files changed, 79 insertions(+), 12 deletions(-) create mode 100644 tests/unit/storage/stats-count-accuracy.test.ts diff --git a/src/storage/baseStorage.ts b/src/storage/baseStorage.ts index 6ba42a7b..c8001e05 100644 --- a/src/storage/baseStorage.ts +++ b/src/storage/baseStorage.ts @@ -2462,18 +2462,27 @@ export abstract class BaseStorage extends BaseStorageAdapter { // 8.0: skip the user-facing total for internal/system entities (counts.json + getNounCount()). if (isNew && metadata.noun && isCounted) { this.incrementEntityCount(metadata.noun) + // Per-type counter (stats().entitiesByType / counts.byTypeEnum) is maintained + // HERE — gated on isNew + visibility, exactly parallel to the total above. It + // used to be bumped unconditionally in saveNoun_internal(), but the HNSW index + // re-saves a node on every neighbor-link change, so that inflated the per-type + // counts with graph connectivity (e.g. 8 documents could read as 44). + this.nounCountsByType[TypeUtils.getNounIndex(metadata.noun as NounType)]++ // Persist counts asynchronously (fire and forget) this.scheduleCountPersist().catch(() => { // Ignore persist errors - will retry on next operation }) } else if (!isNew && metadata.noun && wasCounted !== isCounted) { - // Visibility flipped on update(): move the entity in/out of the user-facing total - // (counts.json / getNounCount()). `nounCountsByType` is gated independently in - // `saveNoun_internal()` off the cache warmed just above, so it is not touched here. + // Visibility flipped on update(): move the entity in/out of the user-facing + // total (counts.json / getNounCount()) AND the per-type counter together, so + // stats().entitiesByType stays consistent with getNounCount(). + const typeIdx = TypeUtils.getNounIndex(metadata.noun as NounType) if (isCounted) { this.incrementEntityCount(metadata.noun) + this.nounCountsByType[typeIdx]++ } else { this.decrementEntityCount(metadata.noun) + if (this.nounCountsByType[typeIdx] > 0) this.nounCountsByType[typeIdx]-- } this.scheduleCountPersist().catch(() => {}) } @@ -3656,15 +3665,13 @@ export abstract class BaseStorage extends BaseStorageAdapter { const type = this.getNounType(noun) const path = getNounVectorPath(noun.id) - // Update type tracking — but only for entities that count toward the - // user-facing per-type stats. `nounVisibilityByIdCache` (warmed by - // `saveNounMetadata_internal()`) flags internal/system ids; public ids are - // absent, so the common case still increments. 8.0 visibility exclusion. + // Per-type stats counter (`nounCountsByType`, read by stats().entitiesByType) + // is maintained in saveNounMetadata_internal(), gated on isNew + visibility — + // NOT here. saveNoun_internal() also runs on HNSW neighbor-link re-saves, so + // incrementing here inflated the per-type counts with graph connectivity. We + // only READ the (externally-maintained) count below to decide when to persist. const typeIndex = TypeUtils.getNounIndex(type) const counted = isCountedVisibility(this.nounVisibilityByIdCache.get(noun.id)) - if (counted) { - this.nounCountsByType[typeIndex]++ - } // Write-cache coherent canonical write await this.writeCanonicalObject(path, noun) diff --git a/tests/unit/brainy-core.unit.test.ts b/tests/unit/brainy-core.unit.test.ts index c6403efe..eb6614e4 100644 --- a/tests/unit/brainy-core.unit.test.ts +++ b/tests/unit/brainy-core.unit.test.ts @@ -202,8 +202,10 @@ describe('Brainy 3.0 Core (Unit Tests)', () => { const stats = await brain.stats() expect(stats.mode).toBe('writer') - expect(stats.entityCount).toBeGreaterThanOrEqual(2) - expect(stats.entitiesByType[NounType.Concept]).toBeGreaterThanOrEqual(2) + // Exact, not >= : the per-type counter must equal the entity count even + // after HNSW re-saves neighbor links (see stats-count-accuracy.test.ts). + expect(stats.entityCount).toBe(2) + expect(stats.entitiesByType[NounType.Concept]).toBe(2) }) }) diff --git a/tests/unit/storage/stats-count-accuracy.test.ts b/tests/unit/storage/stats-count-accuracy.test.ts new file mode 100644 index 00000000..89051f2c --- /dev/null +++ b/tests/unit/storage/stats-count-accuracy.test.ts @@ -0,0 +1,58 @@ +/** + * @module tests/unit/storage/stats-count-accuracy + * @description Regression for per-type count inflation. `nounCountsByType` (read by + * `brain.stats().entitiesByType` and `counts.byTypeEnum`) used to be incremented in + * `saveNoun_internal()`, which runs on EVERY noun write — including the HNSW index + * re-saving a node whenever its neighbor links change. So the per-type counts grew + * with graph connectivity rather than entity count (an internal report saw 8 + * documents read as 44). The counter is now maintained in `saveNounMetadata_internal()` + * gated on `isNew` + visibility, exactly parallel to the authoritative total — so the + * per-type counts must equal the entity count EXACTLY, no matter how dense the graph. + * + * These assert exact equality on purpose: the prior `toBeGreaterThanOrEqual` style + * passed for any inflated value and masked the bug. + */ +import { describe, it, expect } from 'vitest' +import { Brainy } from '../../../src/brainy.js' +import { NounType } from '../../../src/types/graphTypes.js' + +describe('per-type count accuracy (HNSW re-save inflation)', () => { + it('stats()/counts report the EXACT entity count for a single dense type', async () => { + const brain = new Brainy({ requireSubtype: false, storage: { type: 'memory' } }) + await brain.init() + try { + // Enough nodes that HNSW links each new node to existing neighbors and + // re-saves them — the exact path that used to double-count. + const N = 30 + for (let i = 0; i < N; i++) { + await brain.add({ data: { name: `person ${i}` }, type: NounType.Person }) + } + + const stats = await brain.stats() + expect(stats.entityCount).toBe(N) + expect(stats.entitiesByType[NounType.Person]).toBe(N) + expect(brain.counts.byTypeEnum(NounType.Person)).toBe(N) + expect(await brain.getNounCount()).toBe(N) + } finally { + await brain.close() + } + }) + + it('keeps per-type counts exact across multiple types', async () => { + const brain = new Brainy({ requireSubtype: false, storage: { type: 'memory' } }) + await brain.init() + try { + for (let i = 0; i < 12; i++) await brain.add({ data: { n: `p${i}` }, type: NounType.Person }) + for (let i = 0; i < 7; i++) await brain.add({ data: { n: `d${i}` }, type: NounType.Document }) + + const stats = await brain.stats() + expect(stats.entitiesByType[NounType.Person]).toBe(12) + expect(stats.entitiesByType[NounType.Document]).toBe(7) + expect(stats.entityCount).toBe(19) + expect(brain.counts.byTypeEnum(NounType.Person)).toBe(12) + expect(brain.counts.byTypeEnum(NounType.Document)).toBe(7) + } finally { + await brain.close() + } + }) +})