fix(token-meter): reject stale config (round 2)
This commit is contained in:
@@ -273,7 +273,7 @@ estimateMessage(message: Message): number
|
|||||||
|
|
||||||
Types: [Message](../core-data-structures/core.md)
|
Types: [Message](../core-data-structures/core.md)
|
||||||
|
|
||||||
Source: [`packages/llm/token-meter/src/index.ts:92`](../../packages/llm/token-meter/src/index.ts)
|
Source: [`packages/llm/token-meter/src/index.ts:107`](../../packages/llm/token-meter/src/index.ts)
|
||||||
|
|
||||||
## `ctx.tools` — `ToolRegistry`
|
## `ctx.tools` — `ToolRegistry`
|
||||||
|
|
||||||
|
|||||||
@@ -28,7 +28,7 @@ This is not a coupling smell — it is the contract's domain. The "only cordis"
|
|||||||
|
|
||||||
### Abstract `compactIfNeeded` / `compactRegion`, algorithm in the backend
|
### Abstract `compactIfNeeded` / `compactRegion`, algorithm in the backend
|
||||||
|
|
||||||
An earlier draft put the full algorithm (the retention walk, token-summing, text extraction) as concrete methods on the interface. That recouples the contract to one strategy: a backend that wants a different retention policy or event sequence would have to fight inherited concrete code. Making both core methods abstract puts every *how* decision in the backend and keeps the interface a statement of *what*. Token measurement is not a compaction hook at all; the standalone service lets multiple consumers share one model/session replay fold.
|
An earlier draft put the full algorithm (the retention walk, token-summing, text extraction) as concrete methods on the interface. That recouples the contract to one strategy: a backend that wants a different retention policy or event sequence would have to fight inherited concrete code. Making both core methods abstract puts every *how* decision in the backend and keeps the interface a statement of *what*. Token measurement is not a compaction hook at all; the singleton service lets multiple consumers share one per-session replay fold.
|
||||||
|
|
||||||
`compactIfNeeded(agent, fullSystemPrompt, sessionPrefix, signal)` takes required pressure inputs and cancellation. The session comes from the agent. `compactRegion(session, start, end, agent, signal?)` keeps an optional signal for manual callers and requires `session === agent.session`; implementations reject mismatch before model resolution, lock acquisition, summarization, or log mutation. The pre-step integration resolves a provisional model from the latest logged request header, then `AgentOptions.model`; a model-less router-only first step skips pressure because `agent/request` can route later. The default summarizer resolves its model from explicit config, the latest logged routed model, then agent options.
|
`compactIfNeeded(agent, fullSystemPrompt, sessionPrefix, signal)` takes required pressure inputs and cancellation. The session comes from the agent. `compactRegion(session, start, end, agent, signal?)` keeps an optional signal for manual callers and requires `session === agent.session`; implementations reject mismatch before model resolution, lock acquisition, summarization, or log mutation. The pre-step integration resolves a provisional model from the latest logged request header, then `AgentOptions.model`; a model-less router-only first step skips pressure because `agent/request` can route later. The default summarizer resolves its model from explicit config, the latest logged routed model, then agent options.
|
||||||
|
|
||||||
|
|||||||
@@ -20,7 +20,7 @@ This backend owns the compaction policy:
|
|||||||
|
|
||||||
## Config (`BasicCompactConfig`)
|
## Config (`BasicCompactConfig`)
|
||||||
|
|
||||||
Every setting is optional. The pressure and retention policy applies to the token meter's single context window.
|
Every setting is optional. The pressure and retention policy applies to the token meter's single context window. Unrecognized top-level keys are rejected.
|
||||||
|
|
||||||
| Key | Required | Meaning |
|
| Key | Required | Meaning |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
|
|||||||
@@ -14,6 +14,28 @@ const DEFAULT_THRESHOLD_RATIO = 0.8
|
|||||||
/** Default verbatim-tail fraction of the token meter's context window. */
|
/** Default verbatim-tail fraction of the token meter's context window. */
|
||||||
const DEFAULT_RETAIN_RATIO = 0.16
|
const DEFAULT_RETAIN_RATIO = 0.16
|
||||||
|
|
||||||
|
/** Complete public configuration key set. */
|
||||||
|
const BASIC_COMPACT_CONFIG_KEYS: ReadonlySet<string> = new Set([
|
||||||
|
'thresholdRatio',
|
||||||
|
'retainTokens',
|
||||||
|
'summarizationModel',
|
||||||
|
'maxTokens',
|
||||||
|
'compactionRetries',
|
||||||
|
'auto',
|
||||||
|
])
|
||||||
|
|
||||||
|
/** Reject stale or misspelled keys before defaults can hide them. */
|
||||||
|
function validateConfigKeys(config: BasicCompactConfig): void {
|
||||||
|
for (const key of Object.keys(config)) {
|
||||||
|
if (!BASIC_COMPACT_CONFIG_KEYS.has(key)) {
|
||||||
|
throw new Error(
|
||||||
|
`BasicCompactConfig: unknown key "${key}" `
|
||||||
|
+ '(allowed: thresholdRatio, retainTokens, summarizationModel, maxTokens, compactionRetries, auto)',
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Resolve defaults and validate the service-wide compaction policy.
|
* Resolve defaults and validate the service-wide compaction policy.
|
||||||
* @param config - raw compact-basic configuration.
|
* @param config - raw compact-basic configuration.
|
||||||
@@ -24,6 +46,7 @@ export function resolveConfig(
|
|||||||
config: BasicCompactConfig = {},
|
config: BasicCompactConfig = {},
|
||||||
tokenMeter: TokenMeterService,
|
tokenMeter: TokenMeterService,
|
||||||
): ResolvedConfig {
|
): ResolvedConfig {
|
||||||
|
validateConfigKeys(config)
|
||||||
const thresholdRatio = config.thresholdRatio ?? DEFAULT_THRESHOLD_RATIO
|
const thresholdRatio = config.thresholdRatio ?? DEFAULT_THRESHOLD_RATIO
|
||||||
const retainTokens = config.retainTokens
|
const retainTokens = config.retainTokens
|
||||||
?? Math.floor(tokenMeter.contextWindow * DEFAULT_RETAIN_RATIO)
|
?? Math.floor(tokenMeter.contextWindow * DEFAULT_RETAIN_RATIO)
|
||||||
|
|||||||
@@ -163,6 +163,8 @@ describe('compact configuration and defaults', () => {
|
|||||||
[{ thresholdRatio: 1.1 }, /number in \(0, 1\]/],
|
[{ thresholdRatio: 1.1 }, /number in \(0, 1\]/],
|
||||||
[{ retainTokens: -1 }, /non-negative integer/],
|
[{ retainTokens: -1 }, /non-negative integer/],
|
||||||
[{ thresholdRatio: 0.5, retainTokens: 500 }, /less than threshold/],
|
[{ thresholdRatio: 0.5, retainTokens: 500 }, /less than threshold/],
|
||||||
|
[{ models: { [MODEL]: { retainTokens: 10 } } }, /BasicCompactConfig: unknown key "models"/],
|
||||||
|
[{ thresholdRato: 0.5 }, /BasicCompactConfig: unknown key "thresholdRato"/],
|
||||||
] as Array<[unknown, RegExp]>
|
] as Array<[unknown, RegExp]>
|
||||||
|
|
||||||
for (const [config, pattern] of bad) {
|
for (const [config, pattern] of bad) {
|
||||||
|
|||||||
@@ -20,44 +20,75 @@ afterEach(async () => {
|
|||||||
root = undefined
|
root = undefined
|
||||||
})
|
})
|
||||||
|
|
||||||
|
async function loadYaml(lines: readonly string[]): Promise<Context> {
|
||||||
|
root = await mkdtemp(join(tmpdir(), 'dsh-token-meter-loader-'))
|
||||||
|
const configPath = join(root, 'cordis.yml')
|
||||||
|
await writeFile(configPath, [...lines, ''].join('\n'))
|
||||||
|
|
||||||
|
context = new Context()
|
||||||
|
context.baseUrl = pathToFileURL(root).href + '/'
|
||||||
|
await context.plugin(Loader)
|
||||||
|
context.loader.builtins.include = Include
|
||||||
|
const modules = new Map<string, unknown>([
|
||||||
|
['@deepseek-ai/dsh-llm', LlmService],
|
||||||
|
['@deepseek-ai/dsh-token-meter', TokenMeterService],
|
||||||
|
['@deepseek-ai/dsh-compact-basic', BasicCompactService],
|
||||||
|
])
|
||||||
|
context.loader.internal = {
|
||||||
|
version: 'v2',
|
||||||
|
async import(specifier: string) {
|
||||||
|
if (!modules.has(specifier)) throw new Error(`unexpected Loader import: ${specifier}`)
|
||||||
|
return modules.get(specifier)
|
||||||
|
},
|
||||||
|
} as unknown as NonNullable<typeof context.loader.internal>
|
||||||
|
await context.loader.create({
|
||||||
|
name: 'cordis:include',
|
||||||
|
config: { path: pathToFileURL(configPath).href },
|
||||||
|
})
|
||||||
|
await context.loader.await()
|
||||||
|
return context
|
||||||
|
}
|
||||||
|
|
||||||
describe('real Loader composition', () => {
|
describe('real Loader composition', () => {
|
||||||
it('loads the zero-config token-meter then compact-basic YAML pair', async () => {
|
it('loads the flat token-meter and compact-basic YAML shape', async () => {
|
||||||
root = await mkdtemp(join(tmpdir(), 'dsh-token-meter-loader-'))
|
const loaded = await loadYaml([
|
||||||
const configPath = join(root, 'cordis.yml')
|
|
||||||
await writeFile(configPath, [
|
|
||||||
"- name: '@deepseek-ai/dsh-llm'",
|
"- name: '@deepseek-ai/dsh-llm'",
|
||||||
"- name: '@deepseek-ai/dsh-token-meter'",
|
"- name: '@deepseek-ai/dsh-token-meter'",
|
||||||
|
' config:',
|
||||||
|
' contextWindow: 4096',
|
||||||
"- name: '@deepseek-ai/dsh-compact-basic'",
|
"- name: '@deepseek-ai/dsh-compact-basic'",
|
||||||
'',
|
' config:',
|
||||||
].join('\n'))
|
' thresholdRatio: 0.5',
|
||||||
|
' retainTokens: 512',
|
||||||
context = new Context()
|
' auto: false',
|
||||||
context.baseUrl = pathToFileURL(root).href + '/'
|
|
||||||
await context.plugin(Loader)
|
|
||||||
context.loader.builtins.include = Include
|
|
||||||
const modules = new Map<string, unknown>([
|
|
||||||
['@deepseek-ai/dsh-llm', LlmService],
|
|
||||||
['@deepseek-ai/dsh-token-meter', TokenMeterService],
|
|
||||||
['@deepseek-ai/dsh-compact-basic', BasicCompactService],
|
|
||||||
])
|
])
|
||||||
context.loader.internal = {
|
|
||||||
version: 'v2',
|
|
||||||
async import(specifier: string) {
|
|
||||||
if (!modules.has(specifier)) throw new Error(`unexpected Loader import: ${specifier}`)
|
|
||||||
return modules.get(specifier)
|
|
||||||
},
|
|
||||||
} as unknown as NonNullable<typeof context.loader.internal>
|
|
||||||
await context.loader.create({
|
|
||||||
name: 'cordis:include',
|
|
||||||
config: { path: pathToFileURL(configPath).href },
|
|
||||||
})
|
|
||||||
await context.loader.await()
|
|
||||||
|
|
||||||
const unloaded = [...context.loader.entries()]
|
const unloaded = [...loaded.loader.entries()]
|
||||||
.filter(entry => entry.fiber === undefined && !entry.disabled)
|
.filter(entry => entry.fiber === undefined && !entry.disabled)
|
||||||
.map(entry => entry.options.name)
|
.map(entry => entry.options.name)
|
||||||
expect(unloaded).toEqual([])
|
expect(unloaded).toEqual([])
|
||||||
expect(context.tokenMeter.contextWindow).toBe(128_000)
|
expect(loaded.tokenMeter.contextWindow).toBe(4096)
|
||||||
expect(context.get('compact')).toBeInstanceOf(BasicCompactService)
|
expect(loaded.get('compact')).toBeInstanceOf(BasicCompactService)
|
||||||
|
expect((loaded.compact as BasicCompactService).config).toMatchObject({
|
||||||
|
thresholdRatio: 0.5,
|
||||||
|
retainTokens: 512,
|
||||||
|
auto: false,
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
it('rejects stale token-meter config after Schemastery normalization', async () => {
|
||||||
|
context = new Context()
|
||||||
|
await expect(context.plugin(TokenMeterService, {
|
||||||
|
models: { legacy: { contextWindow: 4096 } },
|
||||||
|
} as never)).rejects.toThrow(/TokenMeterConfig: unknown key "models"/)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('rejects stale compact-basic config after Schemastery normalization', async () => {
|
||||||
|
context = new Context()
|
||||||
|
await context.plugin(LlmService)
|
||||||
|
await context.plugin(TokenMeterService)
|
||||||
|
await expect(context.plugin(BasicCompactService, {
|
||||||
|
models: { legacy: { thresholdRatio: 0.5 } },
|
||||||
|
} as never)).rejects.toThrow(/BasicCompactConfig: unknown key "models"/)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ Replay-aware token measurement through the singleton `ctx.tokenMeter` service. I
|
|||||||
|---|---:|---|
|
|---|---:|---|
|
||||||
| `contextWindow` | `128000` | Positive integer service-wide context capacity. |
|
| `contextWindow` | `128000` | Positive integer service-wide context capacity. |
|
||||||
|
|
||||||
The estimator intentionally uses one fixed heuristic: four characters per token plus structural overhead for roles, blocks, and request-envelope fields. `contextWindow` is the only deployment setting. Direct construction validates it; Loader mounts first apply the package's Schemastery shape validation.
|
The estimator intentionally uses one fixed heuristic: four characters per token plus structural overhead for roles, blocks, and request-envelope fields. `contextWindow` is the only deployment setting. Direct construction validates it; Loader mounts first apply the package's Schemastery shape validation. Unrecognized top-level keys are rejected.
|
||||||
|
|
||||||
## Measurement contract
|
## Measurement contract
|
||||||
|
|
||||||
|
|||||||
@@ -23,6 +23,9 @@ export type * from './types.ts'
|
|||||||
/** Default service-wide provider context capacity. */
|
/** Default service-wide provider context capacity. */
|
||||||
const DEFAULT_CONTEXT_WINDOW = 128_000
|
const DEFAULT_CONTEXT_WINDOW = 128_000
|
||||||
|
|
||||||
|
/** Complete public configuration key set. */
|
||||||
|
const TOKEN_METER_CONFIG_KEYS: ReadonlySet<string> = new Set(['contextWindow'])
|
||||||
|
|
||||||
/** Fixed text-density estimate used until exact tokenization is needed. */
|
/** Fixed text-density estimate used until exact tokenization is needed. */
|
||||||
const CHARS_PER_TOKEN = 4
|
const CHARS_PER_TOKEN = 4
|
||||||
|
|
||||||
@@ -69,8 +72,20 @@ function optionalHeaderEquals(
|
|||||||
return headerEquals(left, right)
|
return headerEquals(left, right)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Reject stale or misspelled keys before defaults can hide them. */
|
||||||
|
function validateConfigKeys(config: TokenMeterConfig): void {
|
||||||
|
for (const key of Object.keys(config)) {
|
||||||
|
if (!TOKEN_METER_CONFIG_KEYS.has(key)) {
|
||||||
|
throw new Error(
|
||||||
|
`TokenMeterConfig: unknown key "${key}" (allowed: contextWindow)`,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/** Resolve and validate the one service-wide context capacity. */
|
/** Resolve and validate the one service-wide context capacity. */
|
||||||
function resolveContextWindow(config: TokenMeterConfig): number {
|
function resolveContextWindow(config: TokenMeterConfig): number {
|
||||||
|
validateConfigKeys(config)
|
||||||
const contextWindow = config.contextWindow === undefined
|
const contextWindow = config.contextWindow === undefined
|
||||||
? DEFAULT_CONTEXT_WINDOW
|
? DEFAULT_CONTEXT_WINDOW
|
||||||
: config.contextWindow
|
: config.contextWindow
|
||||||
|
|||||||
@@ -81,6 +81,11 @@ describe('TokenMeterService configuration and registration', () => {
|
|||||||
expect(meter({ contextWindow: 32_000 }).contextWindow).toBe(32_000)
|
expect(meter({ contextWindow: 32_000 }).contextWindow).toBe(32_000)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it.each(['models', 'contextWidow'])('rejects unknown top-level config key %s', (key) => {
|
||||||
|
expect(() => meter({ [key]: {} }))
|
||||||
|
.toThrow(`TokenMeterConfig: unknown key "${key}"`)
|
||||||
|
})
|
||||||
|
|
||||||
it.each([
|
it.each([
|
||||||
{ contextWindow: 0 },
|
{ contextWindow: 0 },
|
||||||
{ contextWindow: -1 },
|
{ contextWindow: -1 },
|
||||||
|
|||||||
Reference in New Issue
Block a user