round 2: address manual compaction review findings
This commit is contained in:
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write packages/compact/compact/README.md
|
||||
README.md: 9c322db998a3179ac96e8fbee26727f3cedef7bb
|
||||
README.zh.md: 2318df4dc5e34d5d35910f957ba75b3eef1488eb
|
||||
README.md: cfb65f2a786dd58d38a7020a8caefeb3d7372f52
|
||||
README.zh.md: e069bea9ef40d2e1ba7beead5b76324cfd56b839
|
||||
|
||||
@@ -54,7 +54,7 @@ The marker pair names lock acquisition and release, not an exclusive event conta
|
||||
|
||||
## Blocking
|
||||
|
||||
Compaction is serialized by one log-recorded lock shared by all entry points. Tail inspection independently finds the latest unmatched `compact/start` and the newest `session/end-seed`. An unmatched start after that boundary is live and reports `busy`; an older unmatched start is stale evidence from a prior process lifecycle and does not block. The same end-seed transition clears the invariant companion's replay trace.
|
||||
Compaction is serialized by one log-recorded lock shared by all entry points. Tail inspection independently finds the latest unmatched `compact/start` and the newest `session/end-seed`. An unmatched start after that boundary is live and reports `busy`; an older unmatched start is stale evidence from a prior process lifecycle and does not block. The same end-seed transition clears the invariant companion's replay trace. A live bracket cannot cross a `turn/start` or `turn/end`; during adoption, repair boundaries in the inherited prefix remain replayable when the later end-seed proves their open bracket stale.
|
||||
|
||||
The lock is the durable bracket, not a `WeakSet`, wrapper mutex, or client-side anchor. `compact/start` is appended synchronously before summarization yields. Every later failure makes exactly one `compact/end { error }` attempt; if that close append itself fails, the unmatched start remains the intentional busy signal and no flush is attempted. A successfully closed manual attempt is flushed even when it reports `changed` or `summary`, preserving the recorded attempt before turn admission is released.
|
||||
|
||||
|
||||
@@ -54,7 +54,7 @@
|
||||
|
||||
## 阻塞
|
||||
|
||||
压缩由所有入口点共享的一个日志记录锁串行化。尾部检查会分别查找最新的未匹配 `compact/start` 和最新的 `session/end-seed`。位于该边界之后的未匹配 start 是活动锁并报告 `busy`;更早的未匹配 start 是先前进程生命周期留下的陈旧证据,不会阻塞。同一个 end-seed 转换会清除不变量配套组件的回放追踪状态。
|
||||
压缩由所有入口点共享的一个日志记录锁串行化。尾部检查会分别查找最新的未匹配 `compact/start` 和最新的 `session/end-seed`。位于该边界之后的未匹配 start 是活动锁并报告 `busy`;更早的未匹配 start 是先前进程生命周期留下的陈旧证据,不会阻塞。同一个 end-seed 转换会清除不变量配套组件的回放追踪状态。活动标记对不能跨越 `turn/start` 或 `turn/end`;在接管会话时,如果后续 end-seed 证明打开的标记对已经陈旧,则继承前缀中的修复边界仍可回放。
|
||||
|
||||
锁就是持久标记对,而非 `WeakSet`、包装层 mutex 或客户端侧锚点。`compact/start` 会在摘要让出控制权之前同步追加。之后每次失败都会恰好尝试一次 `compact/end { error }`;如果追加该闭合事件本身失败,未匹配 start 会继续作为有意保留的 busy 信号,并且不会尝试 flush。已成功闭合的手动尝试即使报告 `changed` 或 `summary` 也会 flush,从而在释放轮次接纳预留前保留该记录。
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ export const name = 'compact-invariant'
|
||||
export const inject = ['invariants']
|
||||
|
||||
interface CompactionTrace {
|
||||
startSeq: number
|
||||
turn: number | null
|
||||
summarized: boolean
|
||||
}
|
||||
@@ -23,11 +24,59 @@ interface SessionTrace {
|
||||
}
|
||||
|
||||
type CompactionTransition =
|
||||
| { kind: 'start'; turn: number | null }
|
||||
| { kind: 'summary'; turn: number | null }
|
||||
| { kind: 'start'; startSeq: number; turn: number | null }
|
||||
| { kind: 'summary'; startSeq: number; turn: number | null }
|
||||
| { kind: 'end' }
|
||||
| { kind: 'end-seed' }
|
||||
|
||||
/** Compaction starts still unmatched when a later seed boundary made them stale. */
|
||||
function inheritedOrphanStartSeqs(
|
||||
events: readonly SessionEvent[],
|
||||
): ReadonlySet<number> {
|
||||
const stale = new Set<number>()
|
||||
let openStartSeq: number | undefined
|
||||
for (const event of events) {
|
||||
if (event.type === 'compact/start') {
|
||||
openStartSeq = event.seq
|
||||
} else if (event.type === 'compact/end') {
|
||||
openStartSeq = undefined
|
||||
} else if (event.type === 'session/end-seed') {
|
||||
if (openStartSeq !== undefined) stale.add(openStartSeq)
|
||||
openStartSeq = undefined
|
||||
}
|
||||
}
|
||||
return stale
|
||||
}
|
||||
|
||||
/** Keep every live compaction bracket on one side of each turn boundary. */
|
||||
function validateTurnBoundary(
|
||||
trace: SessionTrace,
|
||||
event: SessionEvent,
|
||||
fail: InvariantFailure,
|
||||
): void {
|
||||
if (
|
||||
(event.type !== 'turn/start' && event.type !== 'turn/end')
|
||||
|| trace.compaction === undefined
|
||||
) return
|
||||
const owner = trace.compaction.turn === null
|
||||
? 'standalone compaction'
|
||||
: `compaction for turn ${trace.compaction.turn}`
|
||||
fail(`${event.type} cannot cross an open ${owner}`)
|
||||
}
|
||||
|
||||
/** Advance the committed turn cursor after its boundary has been accepted. */
|
||||
function applyTurnBoundary(trace: SessionTrace, event: SessionEvent): boolean {
|
||||
if (event.type === 'turn/start') {
|
||||
trace.openTurn = event.data.turn
|
||||
return true
|
||||
}
|
||||
if (event.type === 'turn/end') {
|
||||
trace.openTurn = null
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/** Require a numbered bracket inside its exact turn, or a standalone bracket between turns. */
|
||||
function validateOwner(
|
||||
owner: number | null,
|
||||
@@ -40,12 +89,7 @@ function validateOwner(
|
||||
return
|
||||
}
|
||||
if (openTurn === null) fail(`${eventType} for turn ${owner} appended outside any open turn`)
|
||||
if (owner !== openTurn) {
|
||||
if (eventType === 'compact/summary') {
|
||||
fail(`compact/summary belongs to turn ${owner} but open turn is ${openTurn}`)
|
||||
}
|
||||
fail(`${eventType} names turn ${owner} but open turn is ${openTurn}`)
|
||||
}
|
||||
if (owner !== openTurn) fail(`${eventType} names turn ${owner} but open turn is ${openTurn}`)
|
||||
}
|
||||
|
||||
/** Validate one compaction event without advancing committed trace state. */
|
||||
@@ -65,7 +109,7 @@ function validateCompactionEvent(
|
||||
fail(`compact/start while ${owner} is still compacting`)
|
||||
}
|
||||
validateOwner(event.data.turn, trace.openTurn, event.type, fail)
|
||||
return { kind: 'start', turn: event.data.turn }
|
||||
return { kind: 'start', startSeq: event.seq, turn: event.data.turn }
|
||||
}
|
||||
if (event.type === 'compact/summary') {
|
||||
if (open === undefined) fail('compact/summary has no matching compact/start')
|
||||
@@ -79,7 +123,7 @@ function validateCompactionEvent(
|
||||
if (!Number.isSafeInteger(event.data.shadowedTokenCount) || event.data.shadowedTokenCount < 0) {
|
||||
fail('compact/summary shadowedTokenCount must be a non-negative safe integer')
|
||||
}
|
||||
return { kind: 'summary', turn: open.turn }
|
||||
return { kind: 'summary', startSeq: open.startSeq, turn: open.turn }
|
||||
}
|
||||
if (open === undefined) fail('compact/end has no matching compact/start')
|
||||
if (event.data.turn !== open.turn) {
|
||||
@@ -96,8 +140,20 @@ function validateCompactionEvent(
|
||||
function applyCompactionTransition(
|
||||
transition: CompactionTransition,
|
||||
): CompactionTrace | undefined {
|
||||
if (transition.kind === 'start') return { turn: transition.turn, summarized: false }
|
||||
if (transition.kind === 'summary') return { turn: transition.turn, summarized: true }
|
||||
if (transition.kind === 'start') {
|
||||
return {
|
||||
startSeq: transition.startSeq,
|
||||
turn: transition.turn,
|
||||
summarized: false,
|
||||
}
|
||||
}
|
||||
if (transition.kind === 'summary') {
|
||||
return {
|
||||
startSeq: transition.startSeq,
|
||||
turn: transition.turn,
|
||||
summarized: true,
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
@@ -110,11 +166,20 @@ const install: InvariantInstaller = Object.assign((ctx: Context, fail: Invariant
|
||||
const seed = (session: Session): SessionTrace => {
|
||||
const trace: SessionTrace = { openTurn: null, compaction: undefined }
|
||||
traces.set(session, trace)
|
||||
const staleOrphanStartSeqs = inheritedOrphanStartSeqs(session.events)
|
||||
for (const event of session.events) {
|
||||
if (event.type === 'turn/start') trace.openTurn = event.data.turn
|
||||
else if (event.type === 'turn/end') trace.openTurn = null
|
||||
// Constructor-seed repair boundaries can precede the end-seed marker
|
||||
// that proves an inherited orphan stale. Replay that inherited prefix
|
||||
// without letting the soon-to-be-cleared bracket veto its repair.
|
||||
if (
|
||||
trace.compaction === undefined
|
||||
|| !staleOrphanStartSeqs.has(trace.compaction.startSeq)
|
||||
) {
|
||||
validateTurnBoundary(trace, event, fail)
|
||||
}
|
||||
const transition = validateCompactionEvent(trace, event, fail)
|
||||
if (transition !== undefined) trace.compaction = applyCompactionTransition(transition)
|
||||
applyTurnBoundary(trace, event)
|
||||
}
|
||||
return trace
|
||||
}
|
||||
@@ -124,14 +189,8 @@ const install: InvariantInstaller = Object.assign((ctx: Context, fail: Invariant
|
||||
ctx.on('session/created', (session) => { seed(session) }, { global: true })
|
||||
ctx.on('session/event', (session, event) => {
|
||||
const trace = traceFor(session)
|
||||
if (event.type === 'turn/start') {
|
||||
trace.openTurn = event.data.turn
|
||||
return
|
||||
}
|
||||
if (event.type === 'turn/end') {
|
||||
trace.openTurn = null
|
||||
return
|
||||
}
|
||||
validateTurnBoundary(trace, event, fail)
|
||||
if (applyTurnBoundary(trace, event)) return
|
||||
if (event.type !== 'session/end-seed'
|
||||
&& event.type !== 'compact/start'
|
||||
&& event.type !== 'compact/summary'
|
||||
@@ -145,7 +204,9 @@ const install: InvariantInstaller = Object.assign((ctx: Context, fail: Invariant
|
||||
ctx.on('internal/dispatch', (_mode, eventName, args) => {
|
||||
if (eventName !== 'session/event') return
|
||||
const [session, event] = args as [Session, SessionEvent]
|
||||
const transition = validateCompactionEvent(traceFor(session), event, fail)
|
||||
const trace = traceFor(session)
|
||||
validateTurnBoundary(trace, event, fail)
|
||||
const transition = validateCompactionEvent(trace, event, fail)
|
||||
if (transition !== undefined) staged.set(event, { session, transition })
|
||||
}, { global: true })
|
||||
}, { inject: ['sessions'] })
|
||||
|
||||
@@ -73,6 +73,71 @@ describe('compaction invariants', () => {
|
||||
}).not.toThrow()
|
||||
})
|
||||
|
||||
it('allows repair turn boundaries after end-seed clears a seeded numbered orphan', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
const source = new Session(SessionId('stale-numbered-compaction-source'))
|
||||
startTurn(source)
|
||||
source.append('compact/start', { turn: 1 })
|
||||
const replayed = ctx.sessions.create(SessionId('stale-numbered-compaction-replay'), {
|
||||
seed: source.events,
|
||||
})
|
||||
expect(replayed.events.map(event => event.type))
|
||||
.toEqual(['turn/start', 'compact/start', 'session/end-seed'])
|
||||
|
||||
await ctx.plugin(InvariantService)
|
||||
await ctx.plugin(CompactInvariant)
|
||||
|
||||
expect(() => replayed.append(
|
||||
'turn/end',
|
||||
{ turn: 1, reason: { kind: 'interrupted' } },
|
||||
)).not.toThrow()
|
||||
})
|
||||
|
||||
it('accepts inherited repair boundaries before the end-seed that clears a standalone orphan', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
const source = new Session(SessionId('stale-repaired-compaction-source'))
|
||||
source.append('compact/start', { turn: null })
|
||||
startTurn(source)
|
||||
source.append('turn/end', { turn: 1, reason: { kind: 'interrupted' } })
|
||||
const replayed = ctx.sessions.create(SessionId('stale-repaired-compaction-replay'), {
|
||||
seed: source.events,
|
||||
})
|
||||
expect(replayed.events.map(event => event.type)).toEqual([
|
||||
'compact/start',
|
||||
'turn/start',
|
||||
'turn/end',
|
||||
'session/end-seed',
|
||||
])
|
||||
|
||||
await ctx.plugin(InvariantService)
|
||||
await expect(ctx.plugin(CompactInvariant).then(() => undefined)).resolves.toBeUndefined()
|
||||
|
||||
expect(() => {
|
||||
startTurn(replayed, 2)
|
||||
replayed.append('turn/end', { turn: 2, reason: { kind: 'completed' } })
|
||||
}).not.toThrow()
|
||||
})
|
||||
|
||||
it('rejects a closed standalone bracket that contains a turn before end-seed', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
const source = new Session(SessionId('closed-nested-compaction-source'))
|
||||
source.append('compact/start', { turn: null })
|
||||
startTurn(source)
|
||||
source.append('turn/end', { turn: 1, reason: { kind: 'interrupted' } })
|
||||
source.append('compact/end', { turn: null, error: 'failed after crossing turn' })
|
||||
const replayed = ctx.sessions.create(SessionId('closed-nested-compaction-replay'), {
|
||||
seed: source.events,
|
||||
})
|
||||
expect(replayed.events.at(-1)?.type).toBe('session/end-seed')
|
||||
|
||||
await ctx.plugin(InvariantService)
|
||||
await expect(ctx.plugin(CompactInvariant).then(() => undefined))
|
||||
.rejects.toThrow(/turn\/start cannot cross an open standalone compaction/)
|
||||
})
|
||||
|
||||
it('rebuilds an open trace when the companion loads after the session', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
@@ -141,23 +206,30 @@ describe('compaction invariants', () => {
|
||||
await expect(ctx.plugin(CompactInvariant).then(() => undefined)).rejects.toThrow(/outside any open turn/)
|
||||
})
|
||||
|
||||
it('rejects an open compaction that crosses into another turn', async () => {
|
||||
it('rejects turn boundaries that cross live standalone or numbered compaction brackets', async () => {
|
||||
const ctx = await setup()
|
||||
const summarySession = ctx.sessions.create()
|
||||
startTurn(summarySession)
|
||||
summarySession.append('compact/start', { turn: 1 })
|
||||
summarySession.append('turn/end', { turn: 1, reason: { kind: 'completed' } })
|
||||
startTurn(summarySession, 2)
|
||||
expect(() => summarySession.append('compact/summary', summary()))
|
||||
.toThrow(/belongs to turn 1 but open turn is 2/)
|
||||
const standalone = ctx.sessions.create()
|
||||
standalone.append('compact/start', { turn: null })
|
||||
expect(() => { startTurn(standalone) })
|
||||
.toThrow(/turn\/start cannot cross an open standalone compaction/)
|
||||
standalone.append('compact/end', { turn: null, error: 'cancelled' })
|
||||
expect(() => {
|
||||
startTurn(standalone)
|
||||
standalone.append('turn/end', { turn: 1, reason: { kind: 'completed' } })
|
||||
}).not.toThrow()
|
||||
|
||||
const endSession = ctx.sessions.create()
|
||||
startTurn(endSession)
|
||||
endSession.append('compact/start', { turn: 1 })
|
||||
endSession.append('turn/end', { turn: 1, reason: { kind: 'completed' } })
|
||||
startTurn(endSession, 2)
|
||||
expect(() => endSession.append('compact/end', { turn: 1, error: 'late' }))
|
||||
.toThrow(/names turn 1 but open turn is 2/)
|
||||
const numbered = ctx.sessions.create()
|
||||
startTurn(numbered)
|
||||
numbered.append('compact/start', { turn: 1 })
|
||||
expect(() => numbered.append(
|
||||
'turn/end',
|
||||
{ turn: 1, reason: { kind: 'completed' } },
|
||||
)).toThrow(/turn\/end cannot cross an open compaction for turn 1/)
|
||||
numbered.append('compact/end', { turn: 1, error: 'cancelled' })
|
||||
expect(() => numbered.append(
|
||||
'turn/end',
|
||||
{ turn: 1, reason: { kind: 'completed' } },
|
||||
)).not.toThrow()
|
||||
})
|
||||
|
||||
it.each([
|
||||
|
||||
Reference in New Issue
Block a user