fix: validate compaction config pairs
This commit is contained in:
@@ -248,17 +248,29 @@ function validatePolicy(
|
||||
assertNonNegativeInteger(`${name}.maxOverflowRetries`, maxOverflowRetries)
|
||||
}
|
||||
|
||||
const summarizationProvider = config.summarizationProvider
|
||||
const summarizationModel = config.summarizationModel
|
||||
if (summarizationProvider !== undefined && typeof summarizationProvider !== 'string') {
|
||||
validateSummarizationPair(config, name)
|
||||
}
|
||||
|
||||
/** Require one scope to omit, clear, or replace the summarization target as a pair. */
|
||||
function validateSummarizationPair(
|
||||
config: CompactPolicyConfig | Record<string, unknown>,
|
||||
name: string,
|
||||
): void {
|
||||
const provider = config.summarizationProvider
|
||||
const model = config.summarizationModel
|
||||
if (provider !== undefined && typeof provider !== 'string') {
|
||||
throw new Error(`${name}.summarizationProvider must be a string`)
|
||||
}
|
||||
if (summarizationModel !== undefined && typeof summarizationModel !== 'string') {
|
||||
if (model !== undefined && typeof model !== 'string') {
|
||||
throw new Error(`${name}.summarizationModel must be a string`)
|
||||
}
|
||||
if (((summarizationProvider ?? '').length === 0)
|
||||
!== ((summarizationModel ?? '').length === 0)) {
|
||||
throw new Error(`${name}: summarizationProvider and summarizationModel must both be empty or both be non-empty`)
|
||||
if (provider === undefined && model === undefined) return
|
||||
if (provider === undefined || model === undefined
|
||||
|| (provider.length === 0) !== (model.length === 0)) {
|
||||
throw new Error(
|
||||
`${name}: summarizationProvider and summarizationModel must be set together `
|
||||
+ 'as an empty or non-empty pair',
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -308,6 +308,41 @@ describe('compact configuration and defaults', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('inherits, clears, and replaces the summarization target as a pair', () => {
|
||||
const config = resolveConfig({
|
||||
summarizationProvider: 'default-provider',
|
||||
summarizationModel: 'default-model',
|
||||
modelPolicies: [
|
||||
{ provider: 'inherit-provider', model: MODEL },
|
||||
{
|
||||
provider: 'clear-provider',
|
||||
model: MODEL,
|
||||
summarizationProvider: '',
|
||||
summarizationModel: '',
|
||||
},
|
||||
{
|
||||
provider: 'replace-provider',
|
||||
model: MODEL,
|
||||
summarizationProvider: 'replacement-provider',
|
||||
summarizationModel: 'replacement-model',
|
||||
},
|
||||
],
|
||||
})
|
||||
|
||||
expect(resolveTargetPolicy(config, { provider: 'inherit-provider', model: MODEL }))
|
||||
.toMatchObject({
|
||||
summarizationProvider: 'default-provider',
|
||||
summarizationModel: 'default-model',
|
||||
})
|
||||
expect(resolveTargetPolicy(config, { provider: 'clear-provider', model: MODEL }))
|
||||
.toMatchObject({ summarizationProvider: '', summarizationModel: '' })
|
||||
expect(resolveTargetPolicy(config, { provider: 'replace-provider', model: MODEL }))
|
||||
.toMatchObject({
|
||||
summarizationProvider: 'replacement-provider',
|
||||
summarizationModel: 'replacement-model',
|
||||
})
|
||||
})
|
||||
|
||||
it('validates common values and pressure-policy invariants', () => {
|
||||
const bad = [
|
||||
[{ maxTokens: 0 }, /maxTokens/],
|
||||
@@ -316,8 +351,10 @@ describe('compact configuration and defaults', () => {
|
||||
[{ auto: 'yes' }, /auto must be a boolean/],
|
||||
[{ summarizationProvider: 1 }, /summarizationProvider must be a string/],
|
||||
[{ summarizationModel: 1 }, /summarizationModel must be a string/],
|
||||
[{ summarizationProvider: MODEL }, /must both be empty or both be non-empty/],
|
||||
[{ summarizationModel: MODEL }, /must both be empty or both be non-empty/],
|
||||
[{ summarizationProvider: MODEL }, /must be set together/],
|
||||
[{ summarizationModel: MODEL }, /must be set together/],
|
||||
[{ summarizationProvider: '' }, /must be set together/],
|
||||
[{ summarizationModel: '' }, /must be set together/],
|
||||
[{ thresholdRatio: 0 }, /number in \(0, 1\]/],
|
||||
[{ thresholdRatio: 1.1 }, /number in \(0, 1\]/],
|
||||
[{ retainRatio: 0.9 }, /retainRatio \(0.9\) must be less than the resolved thresholdRatio \(0.8\)/],
|
||||
@@ -333,6 +370,16 @@ describe('compact configuration and defaults', () => {
|
||||
[{ modelPolicies: [{ provider: MODEL, model: 1 }] }, /model must be a non-empty string/],
|
||||
[{ modelPolicies: [{ provider: MODEL, model: '' }] }, /model must be a non-empty string/],
|
||||
[{ modelPolicies: [{ provider: MODEL, model: MODEL, summarizationProvider: 1 }] }, /summarizationProvider must be a string/],
|
||||
[{
|
||||
summarizationProvider: 'default-provider',
|
||||
summarizationModel: 'default-model',
|
||||
modelPolicies: [{ provider: MODEL, model: MODEL, summarizationModel: '' }],
|
||||
}, /modelPolicies\[0\].*must be set together/],
|
||||
[{
|
||||
summarizationProvider: 'default-provider',
|
||||
summarizationModel: 'default-model',
|
||||
modelPolicies: [{ provider: MODEL, model: MODEL, summarizationProvider: '' }],
|
||||
}, /modelPolicies\[0\].*must be set together/],
|
||||
[{ modelPolicies: [{ provider: MODEL, model: MODEL, retainRatio: 0.2, retainTokens: 100 }] }, /mutually exclusive/],
|
||||
[
|
||||
{ modelPolicies: [{ provider: MODEL, model: MODEL, thresholdRatio: 0.1 }] },
|
||||
|
||||
@@ -133,7 +133,6 @@ async function harness(toolSteps: number): Promise<{ ctx: Context; compact: Repr
|
||||
auto: true,
|
||||
thresholdRatio: 0.5,
|
||||
retainTokens: 50,
|
||||
summarizationModel: '',
|
||||
maxTokens: 8192,
|
||||
compactionRetries: 1,
|
||||
})
|
||||
|
||||
@@ -85,7 +85,7 @@ describe('real Loader composition', () => {
|
||||
context = new Context()
|
||||
await expect(context.plugin(TokenMeterService, {
|
||||
contextWindow: 4096,
|
||||
})).rejects.toThrow(/TokenMeterConfig: unknown key "contextWindow"/)
|
||||
} as never)).rejects.toThrow(/TokenMeterConfig: unknown key "contextWindow"/)
|
||||
})
|
||||
|
||||
it('rejects stale compact-basic config after Schemastery normalization', async () => {
|
||||
@@ -110,4 +110,19 @@ describe('real Loader composition', () => {
|
||||
}],
|
||||
})).rejects.toThrow(/modelPolicies\[0\]: retainRatio \(0.2\).*thresholdRatio \(0.1\)/)
|
||||
})
|
||||
|
||||
it('rejects an incomplete model-policy summarization pair during plugin load', async () => {
|
||||
context = new Context()
|
||||
await context.plugin(LlmService)
|
||||
await context.plugin(TokenMeterService)
|
||||
await expect(context.plugin(BasicCompactService, {
|
||||
summarizationProvider: 'default-provider',
|
||||
summarizationModel: 'default-model',
|
||||
modelPolicies: [{
|
||||
provider: 'test-provider',
|
||||
model: 'test-model',
|
||||
summarizationModel: '',
|
||||
}],
|
||||
})).rejects.toThrow(/modelPolicies\[0\].*must be set together/)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user