From f4ace256485baf5622a76ef06df9ed69267563be Mon Sep 17 00:00:00 2001 From: Hypatia May Date: Fri, 26 Jun 2026 14:40:20 +0800 Subject: [PATCH] fix(compact): correct _extractText surface-order JSDoc; add disposal to the HMR-safety suite (CBR-005, CBR-006) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/compact/compact-basic/src/index.ts | 7 +++++-- .../compact-basic/tests/compact-basic.spec.ts | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/compact/compact-basic/src/index.ts b/packages/compact/compact-basic/src/index.ts index 7648a245b0..479553a17c 100644 --- a/packages/compact/compact-basic/src/index.ts +++ b/packages/compact/compact-basic/src/index.ts @@ -601,8 +601,11 @@ export class BasicCompactService extends CompactService { /** * Extract plain-text conversation from a set of surface node seqs, for - * feeding into the summarization model. Walks events in log order so the - * summary captures chronological flow. + * feeding into the summarization model. Walks the seqs in the order given + * (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 { const lines: string[] = [] diff --git a/packages/compact/compact-basic/tests/compact-basic.spec.ts b/packages/compact/compact-basic/tests/compact-basic.spec.ts index 3a6d9a0360..971aadb9dd 100644 --- a/packages/compact/compact-basic/tests/compact-basic.spec.ts +++ b/packages/compact/compact-basic/tests/compact-basic.spec.ts @@ -735,6 +735,21 @@ describe('BasicCompactService HMR safety', () => { expect(ctx.compact).toBeDefined() 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)', () => { @@ -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') 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() expect(ctx.get('compact')).toBeUndefined() })