fix(find): orderBy is the order on every path, not only the metadata-only one
`find({ where, orderBy })` answered in field order; `find({ query, where,
orderBy })` and `find({ vector, where, orderBy })` answered in SCORE order.
The vector/filter block ranks the fused candidates by score, cuts the page and
returns early — and the tail's orderBy sort sits below that early return, so on
those paths it never ran. Nothing threw and nothing warned: the ordering request
was dropped in silence, and the two paths disagreed about what "ordered by rank"
means.
Where `connected` or `fusion` kept the tail alive the defect changed shape
rather than disappearing. The block had already CUT the page by score, so the
tail ordered the rows relevance had chosen — a correctly sorted page of the
wrong rows.
The early cut fires only once the candidate set reaches offset+limit rows, which
is why it read green for so long: below that threshold the block falls through
and the tail's sort does apply. An ordering that is correct until there is
enough data to matter.
THE LAW: an explicit orderBy displaces score as the ordering key on every path.
The candidate set the path produced is ordered IN FULL and the page is cut from
that ordering — "page last", the graph-first law applied to ordering rather than
to filtering. Score-ranked early paging stays exactly as it was for the default
case, where score IS the requested order.
The pin is differential against the metadata-only path, the one path that always
honoured orderBy. It is sized so the hybrid legs (each bounded at limit*2)
provably cover the filter universe, and that covering is asserted from the leg's
own output rather than assumed — orderBy orders the candidate set, it does not
enlarge it, and the pin claims nothing about recall.
This commit is contained in:
parent
69bda5b7cb
commit
5e720d17ae
2 changed files with 261 additions and 1 deletions
|
|
@ -8486,7 +8486,23 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
// Rank by score (top offset+limit), then drop the offset — identical ordering
|
||||
// to a full `sort((a, b) => b.score - a.score)` + slice, but the native
|
||||
// `sort:topK` provider can compute only the page instead of the full sort.
|
||||
if (results.length >= offset + limit) {
|
||||
//
|
||||
// ONLY when score IS the requested order. An explicit `orderBy` names a
|
||||
// different ordering key, and this block cannot serve it: it ranks by
|
||||
// score and CUTS the page, so the tail's `orderBy` sort below either
|
||||
// never runs at all (the early return, when there is no `connected` /
|
||||
// `fusion` work left) or runs over a page that score already chose —
|
||||
// ordering eight rows relevance picked instead of the eight the field
|
||||
// ordering asks for. Both readings were silent: `find({ query, where,
|
||||
// orderBy })` answered in score order while `find({ where, orderBy })`
|
||||
// answered in field order, and nothing said the request had been dropped.
|
||||
//
|
||||
// With `orderBy` present the candidate set falls through UNCUT to the
|
||||
// tail, which orders it in full and pages that ordering — "page last",
|
||||
// the graph-first law applied to ordering rather than to filtering. The
|
||||
// set is bounded by the legs (the text matches inside the universe plus
|
||||
// the beam walk's `limit * 2`), not by the store.
|
||||
if (!params.orderBy && results.length >= offset + limit) {
|
||||
const k = offset + limit
|
||||
const order = rankIndicesByScore(results.map(r => r.score), k, true)
|
||||
results = reorderByIndices(results, order).slice(offset, k)
|
||||
|
|
|
|||
244
tests/integration/find-orderby-every-path.test.ts
Normal file
244
tests/integration/find-orderby-every-path.test.ts
Normal file
|
|
@ -0,0 +1,244 @@
|
|||
/**
|
||||
* @module tests/integration/find-orderby-every-path
|
||||
* @description `orderBy` IS THE ORDER — on every find() path, not just the
|
||||
* metadata-only one.
|
||||
*
|
||||
* THE DEFECT. `find({ where, orderBy })` (metadata only) answered in field
|
||||
* order. `find({ query, where, orderBy })` and `find({ vector, where, orderBy })`
|
||||
* answered in SCORE order, silently: the vector/filter block ranked the fused
|
||||
* candidates by score, cut the page, and returned early — the tail's `orderBy`
|
||||
* sort sat below that early return and never ran. Nothing threw, nothing warned,
|
||||
* and the two paths disagreed about what "ordered by rank" means. A caller
|
||||
* paging `orderBy: 'rank', order: 'desc'` over a hybrid find got relevance
|
||||
* order wearing an ordering request's clothes.
|
||||
*
|
||||
* Where `connected` or `fusion` kept the tail alive the defect changed shape
|
||||
* rather than disappearing: the block had already CUT the page by score, so the
|
||||
* tail ordered the rows relevance had chosen instead of the rows the ordering
|
||||
* asks for — a correctly sorted page of the wrong rows.
|
||||
*
|
||||
* The early cut fires only once the candidate set reaches `offset + limit`
|
||||
* rows, which is why small fixtures never saw it: below that threshold the
|
||||
* block falls through and the tail's sort does apply. That is the whole shape
|
||||
* of the bug — an ordering that is correct until there is enough data to matter.
|
||||
*
|
||||
* THE LAW. An explicit `orderBy` displaces score as the ordering key on every
|
||||
* path. The candidate set the path produced is ordered IN FULL and the page is
|
||||
* cut from that ordering — the graph-first law's "page last", applied to
|
||||
* ordering rather than to filtering. Score-ranked early paging is for the
|
||||
* default (no `orderBy`) case only, where score IS the requested order.
|
||||
*
|
||||
* THE PIN. Differential, against the metadata-only path — the one path that
|
||||
* always honoured `orderBy`.
|
||||
*
|
||||
* WHAT THE DIFFERENTIAL CAN AND CANNOT CLAIM. `orderBy` orders the candidate
|
||||
* set; it does not enlarge it. The hybrid legs are bounded by construction (the
|
||||
* text leg and the beam walk each take `limit * 2`), so a differential against
|
||||
* the metadata-only path — whose universe is every matching row — is only
|
||||
* meaningful where those bounds provably cover the universe. The fixture is
|
||||
* sized so they do (12 rows, `limit` 6 → a `limit * 2` = 12-row text leg), and
|
||||
* the covering is ASSERTED from the leg's own output rather than assumed. This
|
||||
* pin is about ordering, and it says nothing about recall.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll } from 'vitest'
|
||||
import { Brainy } from '../../src/brainy'
|
||||
import { NounType, VerbType } from '../../src/types/graphTypes'
|
||||
import { resolveEntityId } from '../../src/utils/idNormalization'
|
||||
|
||||
/** Embedding width of the default model — the row vectors must match it. */
|
||||
const DIM = 384
|
||||
|
||||
/** A deterministic, per-row-distinct unit vector (no embedder in the fixture). */
|
||||
function seededVector(seed: number): number[] {
|
||||
const v = new Array<number>(DIM)
|
||||
for (let i = 0; i < DIM; i++) {
|
||||
v[i] = Math.sin((i + 1) * 0.11 + seed * 0.37) * 0.5 + Math.cos((i + 1) * 0.05 + seed * 0.13) * 0.3
|
||||
}
|
||||
const magnitude = Math.sqrt(v.reduce((sum, x) => sum + x * x, 0))
|
||||
return v.map((x) => x / magnitude)
|
||||
}
|
||||
|
||||
/**
|
||||
* Ranks, shuffled — so no scoring order can reproduce them by luck, and the
|
||||
* ordering the pins assert is visibly not the insertion order either.
|
||||
*/
|
||||
const RANKS = [7, 3, 11, 1, 9, 5, 12, 2, 10, 4, 8, 6]
|
||||
const ROWS = RANKS.length
|
||||
/** The page size every pin uses: `limit * 2` covers the whole universe. */
|
||||
const LIMIT = 6
|
||||
/** The neighbour subset — the graph-first universe — and its own page size. */
|
||||
const NEIGHBOURS = 8
|
||||
const GRAPH_LIMIT = 4
|
||||
|
||||
describe('find(): orderBy is the order on every path', () => {
|
||||
let brain: Brainy<any>
|
||||
const QUERY = 'orbital telemetry'
|
||||
const anchor = 'ordering-anchor'
|
||||
const neighbourIds: string[] = []
|
||||
|
||||
beforeAll(async () => {
|
||||
brain = new Brainy({ requireSubtype: false, storage: { type: 'memory' } })
|
||||
await brain.init()
|
||||
|
||||
let seed = 1
|
||||
await brain.add({
|
||||
id: anchor,
|
||||
data: 'ground station anchor record',
|
||||
type: NounType.Thing,
|
||||
metadata: { lane: 'anchor', rank: 0 },
|
||||
vector: seededVector(seed++)
|
||||
})
|
||||
|
||||
for (let i = 0; i < ROWS; i++) {
|
||||
const id = `row-${i}`
|
||||
await brain.add({
|
||||
id,
|
||||
// EVERY row carries both query words, so the text leg reaches all of
|
||||
// them and the hybrid candidate set covers the whole universe.
|
||||
data: `orbital telemetry packet ${i} recorded downlink`,
|
||||
type: NounType.Document,
|
||||
metadata: { lane: 'alpha', rank: RANKS[i] },
|
||||
vector: seededVector(seed++)
|
||||
})
|
||||
if (i < NEIGHBOURS) {
|
||||
await brain.relate({ from: anchor, to: id, type: VerbType.RelatedTo })
|
||||
neighbourIds.push(resolveEntityId(id))
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('the fixture: the hybrid candidate set covers the whole filter universe', async () => {
|
||||
const universe: string[] = await (brain as any).filterIdsBelted({ lane: 'alpha' })
|
||||
expect(universe).toHaveLength(ROWS)
|
||||
|
||||
// The text leg is bounded at `limit * 2`; the fixture is sized so that
|
||||
// bound reaches every row in the universe. This is the precondition the
|
||||
// differential below rests on — asserted from the leg itself.
|
||||
const textScored = await (brain as any).executeTextSearchScored(QUERY, LIMIT * 2, universe)
|
||||
expect(textScored).toHaveLength(ROWS)
|
||||
|
||||
// And the candidate set is large enough to trigger the score-ranked early
|
||||
// cut this pin exists to keep out of an ordered query's way.
|
||||
expect(ROWS).toBeGreaterThanOrEqual(LIMIT)
|
||||
})
|
||||
|
||||
it('metadata-only + orderBy: the reference ordering', async () => {
|
||||
const rows = await brain.find({
|
||||
where: { lane: 'alpha' },
|
||||
orderBy: 'rank',
|
||||
order: 'desc',
|
||||
limit: LIMIT
|
||||
} as any)
|
||||
expect(rows.map((r: any) => r.metadata.rank)).toEqual([12, 11, 10, 9, 8, 7])
|
||||
})
|
||||
|
||||
it('hybrid (query + where) + orderBy: the same page as the metadata-only path', async () => {
|
||||
const params = { where: { lane: 'alpha' }, orderBy: 'rank', order: 'desc' as const, limit: LIMIT }
|
||||
const expected = await brain.find(params as any)
|
||||
const actual = await brain.find({ ...params, query: QUERY } as any)
|
||||
|
||||
expect(actual).toHaveLength(expected.length)
|
||||
expect(actual.map((r: any) => r.id)).toEqual(expected.map((r: any) => r.id))
|
||||
expect(actual.map((r: any) => r.metadata.rank)).toEqual([12, 11, 10, 9, 8, 7])
|
||||
})
|
||||
|
||||
it('hybrid + orderBy asc: the ordering key is honoured in both directions', async () => {
|
||||
const params = { where: { lane: 'alpha' }, orderBy: 'rank', order: 'asc' as const, limit: LIMIT }
|
||||
const expected = await brain.find(params as any)
|
||||
const actual = await brain.find({ ...params, query: QUERY } as any)
|
||||
|
||||
expect(actual.map((r: any) => r.id)).toEqual(expected.map((r: any) => r.id))
|
||||
expect(actual.map((r: any) => r.metadata.rank)).toEqual([1, 2, 3, 4, 5, 6])
|
||||
})
|
||||
|
||||
it('hybrid + orderBy + offset: page two is page two of the ORDERING', async () => {
|
||||
const params = {
|
||||
where: { lane: 'alpha' },
|
||||
orderBy: 'rank',
|
||||
order: 'desc' as const,
|
||||
limit: LIMIT,
|
||||
offset: LIMIT
|
||||
}
|
||||
const expected = await brain.find(params as any)
|
||||
const actual = await brain.find({ ...params, query: QUERY } as any)
|
||||
|
||||
expect(actual).toHaveLength(LIMIT)
|
||||
expect(actual.map((r: any) => r.id)).toEqual(expected.map((r: any) => r.id))
|
||||
expect(actual.map((r: any) => r.metadata.rank)).toEqual([6, 5, 4, 3, 2, 1])
|
||||
})
|
||||
|
||||
it('hybrid + orderBy: paging walks the ordering monotonically, no row twice', async () => {
|
||||
const seen: number[] = []
|
||||
for (let offset = 0; offset < ROWS; offset += LIMIT) {
|
||||
const page = await brain.find({
|
||||
query: QUERY,
|
||||
where: { lane: 'alpha' },
|
||||
orderBy: 'rank',
|
||||
order: 'desc',
|
||||
limit: LIMIT,
|
||||
offset
|
||||
} as any)
|
||||
seen.push(...page.map((r: any) => r.metadata.rank))
|
||||
}
|
||||
expect(seen).toHaveLength(ROWS)
|
||||
expect(new Set(seen).size).toBe(ROWS)
|
||||
// Strictly descending across every page boundary.
|
||||
for (let i = 1; i < seen.length; i++) expect(seen[i]).toBeLessThan(seen[i - 1])
|
||||
})
|
||||
|
||||
it('vector + where + orderBy: field order, not distance order', async () => {
|
||||
// The beam walk takes `limit * 2` = the whole universe here, so the page is
|
||||
// the true top of the ordering — which distance order cannot produce.
|
||||
const rows = await brain.find({
|
||||
vector: seededVector(1000),
|
||||
where: { lane: 'alpha' },
|
||||
orderBy: 'rank',
|
||||
order: 'desc',
|
||||
limit: LIMIT
|
||||
} as any)
|
||||
expect(rows.map((r: any) => r.metadata.rank)).toEqual([12, 11, 10, 9, 8, 7])
|
||||
})
|
||||
|
||||
it('graph-first (query + connected + where) + orderBy: the neighbour set, ordered', async () => {
|
||||
const actual = await brain.find({
|
||||
query: QUERY,
|
||||
connected: { from: anchor, direction: 'out' as const },
|
||||
where: { lane: 'alpha' },
|
||||
orderBy: 'rank',
|
||||
order: 'desc',
|
||||
limit: GRAPH_LIMIT
|
||||
} as any)
|
||||
|
||||
expect(actual).toHaveLength(GRAPH_LIMIT)
|
||||
const neighbours = new Set(neighbourIds)
|
||||
for (const r of actual) expect(neighbours.has(r.id)).toBe(true)
|
||||
|
||||
// The ordering covers the whole neighbour set, so the page holds the
|
||||
// highest ranks AMONG THE NEIGHBOURS — not the ones the score ranking
|
||||
// happened to surface first and the tail then sorted among themselves.
|
||||
const expectedRanks = RANKS.slice(0, NEIGHBOURS)
|
||||
.sort((a, b) => b - a)
|
||||
.slice(0, GRAPH_LIMIT)
|
||||
expect(expectedRanks).toEqual([12, 11, 9, 7])
|
||||
expect(actual.map((r: any) => r.metadata.rank)).toEqual(expectedRanks)
|
||||
})
|
||||
|
||||
it('fusion + orderBy: the ordering survives the fusion rescore', async () => {
|
||||
const actual = await brain.find({
|
||||
query: QUERY,
|
||||
where: { lane: 'alpha' },
|
||||
fusion: 'weighted',
|
||||
orderBy: 'rank',
|
||||
order: 'desc',
|
||||
limit: LIMIT
|
||||
} as any)
|
||||
expect(actual.map((r: any) => r.metadata.rank)).toEqual([12, 11, 10, 9, 8, 7])
|
||||
})
|
||||
|
||||
it('no orderBy: score order still stands (the default is untouched)', async () => {
|
||||
const rows = await brain.find({ query: QUERY, where: { lane: 'alpha' }, limit: LIMIT } as any)
|
||||
expect(rows).toHaveLength(LIMIT)
|
||||
const scores = rows.map((r: any) => r.score)
|
||||
for (let i = 1; i < scores.length; i++) expect(scores[i]).toBeLessThanOrEqual(scores[i - 1])
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue