mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(session-search): keep the index status honest while a sweep has a backlog (#20753)
* fix(session-search): keep the index status honest while a sweep has a backlog A pass stops reading at its wall-clock deadline and records nothing about the candidates it never opened, which is correct: being owed a read is a fact about the row, not an entry in a queue. But a candidate the opening sweep never reached has no row at all, so the store's `due` count cannot see it. The sweep still reported `completed`, the indexer stamped `lastSweepCompletedAt`, and `status()` answered `current` with a backlog of thousands: "Up to date - 130 files indexed", then 630, then more. The read loop now counts what it decided was owed and did not read and hands the number back as `left`; the pass propagates it; the indexer holds the last pass's count, adds it to `filesDue`, reports `indexing` while it is non-zero or a sweep is owed, and no longer stamps a sweep the deadline cut short as complete. * fix(session-search): only an unread backlog keeps the phase at indexing An armed cadence sweep on a drained index is not a backlog, so it no longer flashes the pane to indexing with nothing due. * fix(session-search): stop counting a deferred `due` row twice in filesDue `status()` reports `filesDue` as `stateCounts().due + left`. The read loop incremented `left` for every candidate the deadline cut off, including one whose row already said `due` — and that row is what `stateCounts().due` counts. A sweep that ran out of time therefore reported each already-due transcript twice. `left` now skips a deferred candidate whose row is already `due`. A candidate with no row, and a `current` row whose file moved, still count: those are the backlog no query can see, which is why `left` exists.
This commit is contained in:
@@ -98,25 +98,42 @@ it('resumes into a grown transcript instead of re-reading it whole', async () =>
|
||||
|
||||
// Nothing is recorded about what a deadline cut off, because being owed is a
|
||||
// fact about the row: the file is read on the next pass for the same reason it
|
||||
// was owed on this one.
|
||||
// was owed on this one. The one thing handed back is how many there were, since
|
||||
// a candidate with no row yet is a backlog no query can see.
|
||||
it('leaves what it ran out of time for owed, with nothing written down', async () => {
|
||||
const all = await candidates()
|
||||
const cut = await runSessionSearchIndexPass(store, all, { rows: rows(), overdue: () => true })
|
||||
|
||||
expect(cut.outOfTime).toBe(true)
|
||||
expect(cut).toMatchObject({ outOfTime: true, left: 1 })
|
||||
expect(store.files()).toHaveLength(1)
|
||||
const second = await passOverAll()
|
||||
expect(second.stats.fullParses).toBe(1)
|
||||
expect(store.files()).toHaveLength(2)
|
||||
})
|
||||
|
||||
// A deferred candidate whose row already says `due` is in `stateCounts().due`,
|
||||
// which the status adds `left` to; counting it here would report it twice.
|
||||
it('leaves a deferred candidate out of the count when its row already says due', async () => {
|
||||
await passOverAll()
|
||||
for (const row of store.files()) {
|
||||
store.setFileState(row.path, 'due')
|
||||
}
|
||||
|
||||
const cut = await runSessionSearchIndexPass(store, await candidates(), {
|
||||
rows: rows(),
|
||||
overdue: () => true
|
||||
})
|
||||
|
||||
expect(cut).toMatchObject({ outOfTime: true, left: 0 })
|
||||
})
|
||||
|
||||
// The deadline is never applied before the pass has read anything, so a single
|
||||
// transcript larger than one deadline is read alone rather than starved.
|
||||
it('reads one file even when the deadline has already expired', async () => {
|
||||
const only = (await candidates()).slice(0, 1)
|
||||
const alone = await runSessionSearchIndexPass(store, only, { rows: rows(), overdue: () => true })
|
||||
|
||||
expect(alone.outOfTime).toBe(false)
|
||||
expect(alone).toMatchObject({ outOfTime: false, left: 0 })
|
||||
expect(store.files()).toHaveLength(1)
|
||||
})
|
||||
|
||||
|
||||
@@ -26,21 +26,29 @@ export type SessionSearchIndexPassOptions = {
|
||||
/**
|
||||
* Reads whatever the decide step says is owed, until the deadline.
|
||||
*
|
||||
* Nothing is recorded about what it did not reach. A candidate the deadline cut
|
||||
* off is still owed on the next pass for the same reason it was owed on this
|
||||
* one — its row says so — so there is no queue to keep, nothing to bound, and
|
||||
* nothing to drop. What the reads themselves leave behind is written by the
|
||||
* index consumer onto the rows.
|
||||
* Nothing is recorded about what it did not reach beyond `left`, a count the
|
||||
* caller reports and nothing acts on. A candidate the deadline cut off is still
|
||||
* owed on the next pass for the same reason it was owed on this one — its row
|
||||
* says so — so there is no queue to keep, nothing to bound, and nothing to
|
||||
* drop. What the reads themselves leave behind is written by the index consumer
|
||||
* onto the rows.
|
||||
*
|
||||
* `left` is what makes the backlog sayable: a candidate with no row yet, or one
|
||||
* whose row does not say it is owed, is counted by no `due` query, so without
|
||||
* this the status has no way to tell an index that holds everything from one
|
||||
* that has barely started. Candidates whose row is already `due` are left out,
|
||||
* because the status adds `left` to that same count.
|
||||
*/
|
||||
export async function runSessionSearchIndexPass(
|
||||
store: SessionSearchStore,
|
||||
candidates: readonly SessionFileCandidate[],
|
||||
options: SessionSearchIndexPassOptions
|
||||
): Promise<{ stats: SessionParseStats; outOfTime: boolean }> {
|
||||
): Promise<{ stats: SessionParseStats; outOfTime: boolean; left: number }> {
|
||||
const stats = createSessionParseStats()
|
||||
const cutoffMs = store.retentionCutoff
|
||||
let read = 0
|
||||
let outOfTime = false
|
||||
let left = 0
|
||||
for (const candidate of candidates) {
|
||||
throwIfAiVaultScanCancelled(options.signal)
|
||||
const path = candidate.file.path
|
||||
@@ -61,6 +69,11 @@ export async function runSessionSearchIndexPass(
|
||||
// count of what a pass left is worth more than the microseconds.
|
||||
outOfTime ||= read > 0 && options.overdue?.() === true
|
||||
if (outOfTime) {
|
||||
// A `due` row is already in `stateCounts().due`, which the status adds
|
||||
// this to; counting it here would report the same file twice.
|
||||
if (row?.state !== 'due') {
|
||||
left += 1
|
||||
}
|
||||
continue
|
||||
}
|
||||
// The clock the deadline reads is one the owner may close behind: the read
|
||||
@@ -80,5 +93,5 @@ export async function runSessionSearchIndexPass(
|
||||
)
|
||||
}
|
||||
}
|
||||
return { stats, outOfTime }
|
||||
return { stats, outOfTime, left }
|
||||
}
|
||||
|
||||
@@ -329,7 +329,9 @@ it('reads what one pass has time for and finishes the rest on the next', async (
|
||||
)
|
||||
}
|
||||
await indexer?.reconcile()
|
||||
expect(indexer?.status()).toMatchObject({ filesIndexed: 3, filesDue: 0 })
|
||||
// Two of the four went unread, and neither has a row, so the count it hands
|
||||
// back is the only thing that can say the index is not done.
|
||||
expect(indexer?.status()).toMatchObject({ filesIndexed: 3, filesDue: 2, phase: 'indexing' })
|
||||
|
||||
await indexer?.reconcile()
|
||||
expect(sessionsMatching('deadlined')).toHaveLength(4)
|
||||
@@ -926,14 +928,24 @@ it('stops the opening sweep at its deadline and drains the rest over the passes
|
||||
await writeClaudeTranscript(transcriptPath(session), [`backlogged session ${index}`], session)
|
||||
}
|
||||
await newIndexer(readsPerPass(2)).start()
|
||||
expect(indexer?.status().filesIndexed).toBe(2)
|
||||
// A sweep that ran out of time did not sweep the machine: it says so rather
|
||||
// than stamping itself complete and reporting the three it never opened as
|
||||
// nothing at all.
|
||||
expect(indexer?.status()).toMatchObject({
|
||||
filesIndexed: 2,
|
||||
filesDue: 3,
|
||||
phase: 'indexing',
|
||||
lastSweepCompletedAt: null
|
||||
})
|
||||
|
||||
await nextCycle()
|
||||
expect(indexer?.status().filesIndexed).toBe(4)
|
||||
expect(indexer?.status()).toMatchObject({ filesIndexed: 4, filesDue: 1, phase: 'indexing' })
|
||||
expect(indexer?.status().lastSweepCompletedAt).toBeNull()
|
||||
|
||||
await nextCycle()
|
||||
expect(sessionsMatching('backlogged')).toHaveLength(5)
|
||||
expect(indexer?.status()).toMatchObject({ filesIndexed: 5, filesDue: 0 })
|
||||
expect(indexer?.status()).toMatchObject({ filesIndexed: 5, filesDue: 0, phase: 'current' })
|
||||
expect(indexer?.status().lastSweepCompletedAt).not.toBeNull()
|
||||
})
|
||||
|
||||
// The sweep cadence, with nobody asking for it: a file outside the recency
|
||||
|
||||
@@ -32,7 +32,11 @@ export type SessionSearchIndexStatus = {
|
||||
phase: SessionSearchIndexPhase
|
||||
/** Rows whose content matches the file at the stat the row records. */
|
||||
filesIndexed: number
|
||||
/** Rows owed a whole read: a declined append, or a window that widened. */
|
||||
/**
|
||||
* Files owed a read: rows the index holds and must re-read (a declined
|
||||
* append, a window that widened), plus candidates the last pass ran out of
|
||||
* time for, which have no row to be counted by.
|
||||
*/
|
||||
filesDue: number
|
||||
/** Rows whose last read did not commit. */
|
||||
filesFailed: number
|
||||
@@ -64,8 +68,10 @@ export type SessionSearchIndexStatus = {
|
||||
* `session-search-deleted-sources.ts`.
|
||||
* - `cyclesSinceSweep` and `sweepNext`, which are about the timer rather than
|
||||
* about any file, and mean nothing to a second process.
|
||||
* - `degradedRoots`, `lastReconcileAt` and `lastSweepCompletedAt`: what the last
|
||||
* pass observed, held so `status()` can answer between passes.
|
||||
* - `degradedRoots`, `lastReconcileAt`, `lastSweepCompletedAt` and `left`: what
|
||||
* the last pass observed, held so `status()` can answer between passes.
|
||||
* `left` cannot be a row: a candidate the deadline never reached has no row
|
||||
* yet, which is exactly why no query can see the backlog.
|
||||
* - `lastCounts`, the one cached query result, read only after `close()` so that
|
||||
* describing what happened does not reopen a handle the owner has finished
|
||||
* with. While the indexer is open every call re-queries.
|
||||
@@ -99,6 +105,8 @@ export class SessionSearchIndexer {
|
||||
private degradedRoots: SessionSearchDegradedRoot[] = []
|
||||
private lastReconcileAt: number | null = null
|
||||
private lastSweepCompletedAt: number | null = null
|
||||
/** Candidates the last completed pass was owed and did not read. */
|
||||
private left = 0
|
||||
private lastCounts: SessionSearchStateCounts | null = null
|
||||
private cyclesSinceSweep = 0
|
||||
private sweepNext = false
|
||||
@@ -192,7 +200,7 @@ export class SessionSearchIndexer {
|
||||
return {
|
||||
phase: this.phase(settled),
|
||||
filesIndexed: settled.current,
|
||||
filesDue: settled.due,
|
||||
filesDue: settled.due + this.left,
|
||||
filesFailed: settled.failed,
|
||||
degradedRoots: this.degradedRoots.map((root) => ({ ...root })),
|
||||
lastReconcileAt: this.lastReconcileAt,
|
||||
@@ -234,11 +242,11 @@ export class SessionSearchIndexer {
|
||||
}
|
||||
|
||||
/**
|
||||
* `current` is a claim, so it takes all three: no row owed a read, no row
|
||||
* whose last read failed, and a whole sweep that finished. `idle` is the
|
||||
* other end of it — an indexer nobody started has not promised to index
|
||||
* anything, and calling that `current` would claim an index nobody built is
|
||||
* up to date.
|
||||
* `current` is a claim, so it takes all of it: nothing owed a read by a row,
|
||||
* nothing owed a read that has no row yet, no row whose last read failed,
|
||||
* and a whole sweep that finished. `idle` is the other end of it
|
||||
* — an indexer nobody started has not promised to index anything, and calling
|
||||
* that `current` would claim an index nobody built is up to date.
|
||||
*/
|
||||
private phase(counts: SessionSearchStateCounts): SessionSearchIndexPhase {
|
||||
if (this.closed) {
|
||||
@@ -252,6 +260,10 @@ export class SessionSearchIndexer {
|
||||
if (this.degradedRoots.length > 0 || counts.failed > 0) {
|
||||
return 'degraded'
|
||||
}
|
||||
// Work the rows cannot show: a candidate the deadline cut off has no row.
|
||||
if (this.left > 0) {
|
||||
return 'indexing'
|
||||
}
|
||||
return counts.due === 0 && this.lastSweepCompletedAt !== null ? 'current' : 'indexing'
|
||||
}
|
||||
|
||||
@@ -303,12 +315,20 @@ export class SessionSearchIndexer {
|
||||
this.degradedRoots = result.degradedRoots
|
||||
this.previousRootsWithFiles = result.rootsWithFiles
|
||||
this.lastReconcileAt = this.clock.now()
|
||||
// Replaced, not accumulated: it is this pass's measure of the backlog, and
|
||||
// a pass that read everything it was owed measures zero.
|
||||
this.left = result.left
|
||||
// A backlog outside the recency window is only visible to a sweep, so a
|
||||
// pass that ran out of time asks for one. It is self-limiting: the first
|
||||
// pass that finishes its reads hands the interval back to cycles.
|
||||
this.sweepNext ||= result.outOfTime
|
||||
if (full) {
|
||||
this.lastSweepCompletedAt = this.lastReconcileAt
|
||||
// A sweep the deadline stopped with candidates still unread did not
|
||||
// sweep the machine, and stamping it would let `current` be claimed
|
||||
// over a backlog no row can account for.
|
||||
if (!result.outOfTime) {
|
||||
this.lastSweepCompletedAt = this.lastReconcileAt
|
||||
}
|
||||
this.cyclesSinceSweep = 0
|
||||
return
|
||||
}
|
||||
|
||||
@@ -75,6 +75,14 @@ export type SessionSearchPassResult = {
|
||||
* making progress only on the periodic sweep every five minutes.
|
||||
*/
|
||||
outOfTime: boolean
|
||||
/**
|
||||
* Candidates this pass decided were owed a read and did not read.
|
||||
*
|
||||
* Zero unless the deadline stopped the reads. Not a queue: it is the size of
|
||||
* the backlog at the moment the pass gave up, reported so the caller can say
|
||||
* so, and every one of them is owed again on the next pass by its row.
|
||||
*/
|
||||
left: number
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -113,6 +121,7 @@ export async function runSessionSearchPass(
|
||||
|
||||
let completed = true
|
||||
let outOfTime = false
|
||||
let left = 0
|
||||
const rows = new Map(store.files().map((row) => [row.path, row]))
|
||||
try {
|
||||
const read = await runSessionSearchIndexPass(store, swept.candidates, {
|
||||
@@ -121,6 +130,7 @@ export async function runSessionSearchPass(
|
||||
overdue: args.overdue
|
||||
})
|
||||
outOfTime = read.outOfTime
|
||||
left = read.left
|
||||
} catch (error) {
|
||||
if (!signal?.aborted) {
|
||||
throw error
|
||||
@@ -183,7 +193,8 @@ export async function runSessionSearchPass(
|
||||
unlistable
|
||||
),
|
||||
completed,
|
||||
outOfTime
|
||||
outOfTime,
|
||||
left
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user