taodeng/olp
1
0
Fork 0
mirror of https://github.com/dtzp555-max/olp.git synced 2026-07-21 21:15:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(cache)+docs(adr-0005): D15 — expand cache key composition to include max_tokens/top_p/stop/tool_choice

Browse Source 
cold-audit catch from 2026-05-23

Cold-audit Finding 7 (P2 cache correctness). ADR 0005 § Cache key
composition (v1.0) listed 7 fields. ADR 0003 § Optional fields added
`max_tokens`, `top_p`, `stop`, `tool_choice` as IR-carried fields
that affect model output. Pre-D15 cache key omitted those 4 —
identical IRs differing only in those fields collided on the same
cache key but produced legitimately different outputs. Concrete
hazard: a request with `max_tokens: 100` could receive a cached
response generated by `max_tokens: 4000` — wrong content (truncated
or unexpectedly extended).

Coordinated change across two layers, single commit per the ADR-with-
code pattern established by D11:

1. docs/adr/0005-cache-cross-provider.md — Amendment 2 (top of doc,
   matching D11 Amendment 1 placement convention)
   - Documents the 4 missing fields + concrete failure mode
   - Expands the v1.0 cache key composition spec
   - Documents the `?? null` collapsing semantics (explicit-null
     equals absent — consistent with existing temperature/response_format)
   - Adds forward-looking note: future IR field additions must be
     evaluated for cache-key inclusion at addition time; default is
     include unless explicit rationale documents safe omission

2. lib/cache/keys.mjs — append the 4 fields to `keyObj` after the
   existing 7 (preserves field ordering; pre-D15 cache entries are
   forced misses on first request — schema forward-compatible)
   - `max_tokens: ir.max_tokens ?? null`
   - `top_p: ir.top_p ?? null`
   - `stop: ir.stop ?? null`
   - `tool_choice: ir.tool_choice ?? null`
   - Updated file-level docblock + function-level JSDoc + @param ir
     enumeration to reflect new schema (the @param fix also closes a
     pre-existing partial-list issue that omitted cache_control)

3. test-features.mjs — 5 new tests in Suite 9:
   - max_tokens differs → different key
   - top_p differs → different key
   - stop differs → different key
   - tool_choice differs → different key (covers string-form
     `'auto'` vs `'none'`; object-form coverage left for future
     defense-in-depth — slot existence is verified by the string case)
   - Both-absent stability check (`?? null` collapsing produces
     identical keys for omitted-vs-omitted)

Tests: 292 → 297 (+5). No hash constants pinned in existing tests
(grep verified) so no pre-D15 tests required updating; assertions
were already shape-based (`assert.equal` / `assert.notEqual` on
hash strings, never specific hex values).

Authority:
- ADR 0003 § Optional fields — source of the 4 IR field definitions
  https://github.com/dtzp555-max/olp/blob/main/docs/adr/0003-intermediate-representation.md
- ADR 0005's stated invariant: "different output → different cache
  entry" — the addition restores compliance with this invariant
- OpenAI /v1/chat/completions spec — confirms the 4 fields affect
  output:
  https://platform.openai.com/docs/api-reference/chat/create
- ALIGNMENT.md Rule 2(c) spirit — ADR amendment + code change land
  in same merge (D11 precedent established this pattern for
  Provider-contract changes; D15 applies same pattern for cache-key
  schema changes)
- CC 开发铁律 v1.6 § 10.x — Cold Audit caught this; diff-review pass
  that approved original ADR 0005 did not cross-reference all ADR
  0003 optional fields

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Folded the one minor (`@param ir`
JSDoc stale partial list) before commit — same JSDoc precision logic
that drove D11's B1 fold-in. Two remaining non-blocking suggestions
(tool_choice object-form test coverage; D11-vs-D15 amendment structure
template) tracked as defense-in-depth opportunities, not required for
spec compliance.

Reviewer's highest-value verification: field ordering preserved.
keyObj at lib/cache/keys.mjs:203-217 reads exactly:
`{provider, model, messages, tools, temperature, response_format,
cache_control, max_tokens, top_p, stop, tool_choice}` — existing 7
in unchanged positions, 4 new appended at end. JSON.stringify on
Node 18+ preserves insertion order; SHA-256 hash deterministic
across runs.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
taodeng
2026-05-24 11:24:12 +10:00
co-authored by Claude Opus 4.7
parent a7085d9718
commit 8ae77c3ae3
3 changed files with 79 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/adr/0005-cache-cross-provider.md
+16 -1
View File
@@ -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
}))
```