fix(8.0): stats() per-type counts no longer inflate with HNSW re-saves
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.
This commit is contained in:
parent
b2005ff22a
commit
21d02d3bae
3 changed files with 79 additions and 12 deletions
|
|
@ -2462,18 +2462,27 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
||||||
// 8.0: skip the user-facing total for internal/system entities (counts.json + getNounCount()).
|
// 8.0: skip the user-facing total for internal/system entities (counts.json + getNounCount()).
|
||||||
if (isNew && metadata.noun && isCounted) {
|
if (isNew && metadata.noun && isCounted) {
|
||||||
this.incrementEntityCount(metadata.noun)
|
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)
|
// Persist counts asynchronously (fire and forget)
|
||||||
this.scheduleCountPersist().catch(() => {
|
this.scheduleCountPersist().catch(() => {
|
||||||
// Ignore persist errors - will retry on next operation
|
// Ignore persist errors - will retry on next operation
|
||||||
})
|
})
|
||||||
} else if (!isNew && metadata.noun && wasCounted !== isCounted) {
|
} else if (!isNew && metadata.noun && wasCounted !== isCounted) {
|
||||||
// Visibility flipped on update(): move the entity in/out of the user-facing total
|
// Visibility flipped on update(): move the entity in/out of the user-facing
|
||||||
// (counts.json / getNounCount()). `nounCountsByType` is gated independently in
|
// total (counts.json / getNounCount()) AND the per-type counter together, so
|
||||||
// `saveNoun_internal()` off the cache warmed just above, so it is not touched here.
|
// stats().entitiesByType stays consistent with getNounCount().
|
||||||
|
const typeIdx = TypeUtils.getNounIndex(metadata.noun as NounType)
|
||||||
if (isCounted) {
|
if (isCounted) {
|
||||||
this.incrementEntityCount(metadata.noun)
|
this.incrementEntityCount(metadata.noun)
|
||||||
|
this.nounCountsByType[typeIdx]++
|
||||||
} else {
|
} else {
|
||||||
this.decrementEntityCount(metadata.noun)
|
this.decrementEntityCount(metadata.noun)
|
||||||
|
if (this.nounCountsByType[typeIdx] > 0) this.nounCountsByType[typeIdx]--
|
||||||
}
|
}
|
||||||
this.scheduleCountPersist().catch(() => {})
|
this.scheduleCountPersist().catch(() => {})
|
||||||
}
|
}
|
||||||
|
|
@ -3656,15 +3665,13 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
||||||
const type = this.getNounType(noun)
|
const type = this.getNounType(noun)
|
||||||
const path = getNounVectorPath(noun.id)
|
const path = getNounVectorPath(noun.id)
|
||||||
|
|
||||||
// Update type tracking — but only for entities that count toward the
|
// Per-type stats counter (`nounCountsByType`, read by stats().entitiesByType)
|
||||||
// user-facing per-type stats. `nounVisibilityByIdCache` (warmed by
|
// is maintained in saveNounMetadata_internal(), gated on isNew + visibility —
|
||||||
// `saveNounMetadata_internal()`) flags internal/system ids; public ids are
|
// NOT here. saveNoun_internal() also runs on HNSW neighbor-link re-saves, so
|
||||||
// absent, so the common case still increments. 8.0 visibility exclusion.
|
// 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 typeIndex = TypeUtils.getNounIndex(type)
|
||||||
const counted = isCountedVisibility(this.nounVisibilityByIdCache.get(noun.id))
|
const counted = isCountedVisibility(this.nounVisibilityByIdCache.get(noun.id))
|
||||||
if (counted) {
|
|
||||||
this.nounCountsByType[typeIndex]++
|
|
||||||
}
|
|
||||||
|
|
||||||
// Write-cache coherent canonical write
|
// Write-cache coherent canonical write
|
||||||
await this.writeCanonicalObject(path, noun)
|
await this.writeCanonicalObject(path, noun)
|
||||||
|
|
|
||||||
|
|
@ -202,8 +202,10 @@ describe('Brainy 3.0 Core (Unit Tests)', () => {
|
||||||
|
|
||||||
const stats = await brain.stats()
|
const stats = await brain.stats()
|
||||||
expect(stats.mode).toBe('writer')
|
expect(stats.mode).toBe('writer')
|
||||||
expect(stats.entityCount).toBeGreaterThanOrEqual(2)
|
// Exact, not >= : the per-type counter must equal the entity count even
|
||||||
expect(stats.entitiesByType[NounType.Concept]).toBeGreaterThanOrEqual(2)
|
// after HNSW re-saves neighbor links (see stats-count-accuracy.test.ts).
|
||||||
|
expect(stats.entityCount).toBe(2)
|
||||||
|
expect(stats.entitiesByType[NounType.Concept]).toBe(2)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|
|
||||||
58
tests/unit/storage/stats-count-accuracy.test.ts
Normal file
58
tests/unit/storage/stats-count-accuracy.test.ts
Normal file
|
|
@ -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()
|
||||||
|
}
|
||||||
|
})
|
||||||
|
})
|
||||||
Loading…
Add table
Add a link
Reference in a new issue