diff --git a/docs/adr/0005-cache-cross-provider.md b/docs/adr/0005-cache-cross-provider.md index 02c351f..a9cfc08 100644 --- a/docs/adr/0005-cache-cross-provider.md +++ b/docs/adr/0005-cache-cross-provider.md @@ -5,6 +5,16 @@ - **Authors:** project maintainer (with AI drafting assistance) - **Related:** OLP v0.1 spec §4.4; ADR 0002 (plugin architecture); ADR 0003 (IR — cache keys are computed over IR shape); ADR 0004 (fallback — cross-provider cache misses are correct on fallback) +## Amendments + +### Amendment 2 — 2026-05-24: Expand cache key composition to include `max_tokens`, `top_p`, `stop`, `tool_choice` (D15) + +- **Finding:** Cold-audit Finding 7 (P2 cache correctness) — the v1.0 cache key composition listed in § "Cache key composition (v1.0)" omitted four IR fields that materially affect model output: `max_tokens` (output length truncation), `top_p` (sampling distribution), `stop` (stop sequences that terminate generation), and `tool_choice` (`'auto' | 'none' | 'required' | {type, function:{name}}` — fundamentally changes whether/which tool the model calls). Two requests identical except for one of these four fields produce different model outputs but collided on the same cache key under v1.0. Consequence: a request with `max_tokens: 100` could receive a cached response originally generated by a `max_tokens: 4000` request — wrong content (truncated or unexpectedly extended). +- **Change:** Expand cache key composition to include `max_tokens`, `top_p`, `stop`, and `tool_choice` in addition to the existing seven fields. The four new fields are appended after the existing set so that the ordering of existing fields is stable (pre-D15 cache entries are invalidated on first request — a forced miss — but the schema is forward-compatible). Field serialization follows the existing `?? null` null-coalescing pattern: a request with `max_tokens: undefined` serializes as `null` and is treated as equivalent to an absent field; `max_tokens: 100` serializes as `100`. This is consistent with how `temperature` and `response_format` are handled. +- **Rationale:** ADR 0005's own stated invariant — "different output → different cache entry" — requires that any IR field affecting output be included in the cache key. The original v1.0 key composition was correct for the seven named fields but provided incomplete coverage of ADR 0003's IR field set. The four omitted fields are all defined in ADR 0003 § Optional fields and are all sourced directly from the OpenAI `/v1/chat/completions` spec parameters that affect generation. +- **Forward-looking note:** Future IR field additions (e.g., `seed`, `frequency_penalty`, `presence_penalty`, `logit_bias`, `logprobs`, `n`) must be evaluated for cache-key inclusion at the time they are added to the IR. The default is to **include** the field in the cache key unless an explicit rationale documents why omitting it is safe (e.g., the field has no effect on the provider's output for any value, or it is a purely client-side metadata field). This evaluation must appear in the amending ADR per ALIGNMENT.md Rule 2(c) spirit. +- **Procedural mechanism:** CC 开发铁律 v1.6 § 10.x (Cold Audit caught it). This follows the calibration-loop precedent established by D11 / Amendment 1 of ADR 0002: the diff-review pass that approved the original ADR 0005 did not cross-reference all ADR 0003 optional fields; the cold-audit pass on 2026-05-23 caught the gap as Finding 7. + ## Context OCP shipped a four-layer cache hardening (D1+D2+D3+D4) in v3.13.0 (2026-05-07 per the MEMORY.md entry). The four layers: @@ -31,7 +41,7 @@ The third design point: `cache_control` (D2) is Anthropic-specific in v1.0. Othe Per spec §4.4, OLP's cache layer: -**Cache key composition (v1.0).** +**Cache key composition (v1.0).** (amended 2026-05-24, see Amendment 2) ``` key = sha256(JSON.stringify({ provider, // e.g., 'anthropic', 'openai', 'mistral' @@ -41,6 +51,11 @@ key = sha256(JSON.stringify({ temperature, // included if non-default response_format, // included if present cache_control, // Anthropic prompt-caching markers (the markers themselves; the bypass logic is separate) + // Added by Amendment 2 (D15) — fields from ADR 0003 § Optional fields that affect output: + max_tokens, // output length truncation; null if absent + top_p, // sampling distribution; null if absent + stop, // stop sequences; null if absent + tool_choice, // 'auto' | 'none' | 'required' | {type, function:{name}}; null if absent })) ``` diff --git a/lib/cache/keys.mjs b/lib/cache/keys.mjs index 297cfc1..d115973 100644 --- a/lib/cache/keys.mjs +++ b/lib/cache/keys.mjs @@ -1,7 +1,7 @@ /** * lib/cache/keys.mjs — Cache key generation for OLP * - * Authority: ADR 0005 § Cache key composition (v1.0) + * Authority: ADR 0005 § Cache key composition (v1.0) — amended 2026-05-24 by Amendment 2 (D15) * * Cache keys are computed over the IR (ADR 0003), not over the raw OpenAI * request shape, so key stability is governed by OLP's own release cadence. @@ -15,6 +15,11 @@ * temperature: ir.temperature ?? null, * response_format: ir.response_format ?? null, * cache_control: extractCacheControlMarkers(ir.messages), + * // Added by D15 (Amendment 2) — ADR 0003 § Optional fields that affect output: + * max_tokens: ir.max_tokens ?? null, + * top_p: ir.top_p ?? null, + * stop: ir.stop ?? null, + * tool_choice: ir.tool_choice ?? null, * })) * * Per ADR 0005 § Per-model isolation: (provider, model) is part of the key. @@ -162,9 +167,10 @@ export function hasCacheControl(ir) { /** * Computes a content-addressed SHA-256 cache key over the IR request. * - * Per ADR 0005 § Cache key composition (v1.0): + * Per ADR 0005 § Cache key composition (v1.0) — amended 2026-05-24 by Amendment 2 (D15): * key = sha256(JSON.stringify({ - * provider, model, messages, tools, temperature, response_format, cache_control + * provider, model, messages, tools, temperature, response_format, cache_control, + * max_tokens, top_p, stop, tool_choice * })) * * The key is deterministic: same inputs → same key. No random, no timestamp. @@ -174,7 +180,7 @@ export function hasCacheControl(ir) { * * @param {string} provider — provider name, e.g. 'anthropic' * @param {string} model — model string, e.g. 'claude-haiku-4-5' - * @param {object} ir — IR request (ADR 0003): { messages, tools?, temperature?, response_format? } + * @param {object} ir — IR request (ADR 0003): { messages, tools?, temperature?, response_format?, cache_control?, max_tokens?, top_p?, stop?, tool_choice? } * @param {object} [_options] — reserved for future options (not used at D5) * @returns {string} — 64-character hex SHA-256 digest */ @@ -202,6 +208,12 @@ export function computeCacheKey(provider, model, ir, _options = {}) { temperature: ir.temperature ?? null, response_format: ir.response_format ?? null, cache_control: cacheControlMarkers.length > 0 ? cacheControlMarkers : null, + // D15 (ADR 0005 Amendment 2): IR fields from ADR 0003 § Optional fields that affect output. + // Appended after the existing seven fields to avoid invalidating ordering-dependent keys. + max_tokens: ir.max_tokens ?? null, + top_p: ir.top_p ?? null, + stop: ir.stop ?? null, + tool_choice: ir.tool_choice ?? null, }; return createHash('sha256').update(JSON.stringify(keyObj)).digest('hex'); diff --git a/test-features.mjs b/test-features.mjs index 020ed90..a5c193c 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -1206,6 +1206,53 @@ describe('Cache layer — computeCacheKey (Suite 9)', () => { const k2 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir2); assert.equal(k1, k2); }); + + // ── D15 Tests: Amendment 2 — max_tokens, top_p, stop, tool_choice ──── + + // ── Test 9: max_tokens distinguishes ───────────────────────────────── + it('computeCacheKey differs when max_tokens differs (D15 Amendment 2)', () => { + const ir1 = makeIR({ messages: [{ role: 'user', content: 'x' }], max_tokens: 100 }); + const ir2 = makeIR({ messages: [{ role: 'user', content: 'x' }], max_tokens: 4000 }); + const k1 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir1); + const k2 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir2); + assert.notEqual(k1, k2); + }); + + // ── Test 10: top_p distinguishes ────────────────────────────────────── + it('computeCacheKey differs when top_p differs (D15 Amendment 2)', () => { + const ir1 = makeIR({ messages: [{ role: 'user', content: 'x' }], top_p: 0.5 }); + const ir2 = makeIR({ messages: [{ role: 'user', content: 'x' }], top_p: 0.9 }); + const k1 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir1); + const k2 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir2); + assert.notEqual(k1, k2); + }); + + // ── Test 11: stop distinguishes ─────────────────────────────────────── + it('computeCacheKey differs when stop sequences differ (D15 Amendment 2)', () => { + const ir1 = makeIR({ messages: [{ role: 'user', content: 'x' }], stop: ['\n'] }); + const ir2 = makeIR({ messages: [{ role: 'user', content: 'x' }], stop: ['END', '\n'] }); + const k1 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir1); + const k2 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir2); + assert.notEqual(k1, k2); + }); + + // ── Test 12: tool_choice distinguishes ─────────────────────────────── + it('computeCacheKey differs when tool_choice differs (D15 Amendment 2)', () => { + const ir1 = makeIR({ messages: [{ role: 'user', content: 'x' }], tool_choice: 'auto' }); + const ir2 = makeIR({ messages: [{ role: 'user', content: 'x' }], tool_choice: 'none' }); + const k1 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir1); + const k2 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir2); + assert.notEqual(k1, k2); + }); + + // ── Test 13: undefined max_tokens stable (both absent → same key) ──── + it('computeCacheKey is identical when max_tokens absent in both requests (D15 Amendment 2)', () => { + const ir1 = makeIR({ messages: [{ role: 'user', content: 'x' }] }); + const ir2 = makeIR({ messages: [{ role: 'user', content: 'x' }] }); + const k1 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir1); + const k2 = computeCacheKey('anthropic', 'claude-haiku-4-5', ir2); + assert.equal(k1, k2); + }); }); describe('Cache layer — extractCacheControlMarkers + hasCacheControl (Suite 9 cont.)', () => {