fix(compact): correct _extractText surface-order JSDoc; add disposal to the HMR-safety suite (CBR-005, CBR-006)
Manual review round, two non-blocking findings:
- CBR-005: _extractText's JSDoc claimed it "walks events in log order",
but it walks the seqs in surface order (the inline comment already said
so) — the exact distinction CBR-001 paid for, since after a replace a
high-seq checkpoint heads the surface before lower-seq retained nodes.
Corrected the JSDoc to match.
- CBR-006: the "HMR safety" suite only asserted registration; the actual
dispose-and-confirm-cleanup test lived under "llm inject", so a reader
searching by name could miss it. Added a disposal test to the HMR-safety
suite (mount via the real plugin fiber with LlmService present so inject
resolves, dispose, assert ctx.get('compact') is undefined) and reframed
the llm-inject test's trailing teardown to point at it.
This commit is contained in:
@@ -601,8 +601,11 @@ export class BasicCompactService extends CompactService {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Extract plain-text conversation from a set of surface node seqs, for
|
* Extract plain-text conversation from a set of surface node seqs, for
|
||||||
* feeding into the summarization model. Walks events in log order so the
|
* feeding into the summarization model. Walks the seqs in the order given
|
||||||
* summary captures chronological flow.
|
* (surface order, as `compactRegion` slices the surface-node list) so the
|
||||||
|
* summary follows the conversation as the model sees it — which, after a
|
||||||
|
* `replace`, is NOT ascending log-seq order (a high-seq summary node heads the
|
||||||
|
* surface before older retained lower-seq nodes).
|
||||||
*/
|
*/
|
||||||
private _extractText(session: Session, seqs: number[]): string {
|
private _extractText(session: Session, seqs: number[]): string {
|
||||||
const lines: string[] = []
|
const lines: string[] = []
|
||||||
|
|||||||
@@ -735,6 +735,21 @@ describe('BasicCompactService HMR safety', () => {
|
|||||||
expect(ctx.compact).toBeDefined()
|
expect(ctx.compact).toBeDefined()
|
||||||
expect(ctx.compact).toBeInstanceOf(BasicCompactService)
|
expect(ctx.compact).toBeInstanceOf(BasicCompactService)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('disposing the plugin fiber unregisters ctx.compact', async () => {
|
||||||
|
// Mount through the real plugin fiber (the Loader path), then dispose it and
|
||||||
|
// confirm the service registration is torn down. LlmService is mounted first
|
||||||
|
// so the service's `inject: ['llm']` resolves and the fiber activates. (The
|
||||||
|
// sibling-fiber ctx.llm resolution this same setup also exercises is covered
|
||||||
|
// under the "llm inject (real plugin-load path)" suite.)
|
||||||
|
const ctx = new Context()
|
||||||
|
await ctx.plugin(LlmService)
|
||||||
|
const fiber = await ctx.plugin(BasicCompactService, { auto: false })
|
||||||
|
expect(ctx.get('compact')).toBeInstanceOf(BasicCompactService)
|
||||||
|
|
||||||
|
await fiber.dispose()
|
||||||
|
expect(ctx.get('compact')).toBeUndefined()
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe('BasicCompactService convergence invariant (config)', () => {
|
describe('BasicCompactService convergence invariant (config)', () => {
|
||||||
@@ -1343,7 +1358,8 @@ describe('BasicCompactService llm inject (real plugin-load path)', () => {
|
|||||||
const result = await svc.compactRegion(session, nodes[0]!.seq, nodes[1]!.seq, 'test-model')
|
const result = await svc.compactRegion(session, nodes[0]!.seq, nodes[1]!.seq, 'test-model')
|
||||||
expect(result.summary).toEqual([{ type: 'text', text: 'CONDENSED' }])
|
expect(result.summary).toEqual([{ type: 'text', text: 'CONDENSED' }])
|
||||||
|
|
||||||
// HMR: disposing the fiber tears the service registration down.
|
// Tear the fiber down so this test owns no leaked registration; the
|
||||||
|
// dedicated cleanup assertion lives in the "HMR safety" suite.
|
||||||
await fiber.dispose()
|
await fiber.dispose()
|
||||||
expect(ctx.get('compact')).toBeUndefined()
|
expect(ctx.get('compact')).toBeUndefined()
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user