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

feat+docs+test: D40 — X-OLP-Fallback-Detail header (issue #7)

Browse Source 
ADR 0004 § Decision § Chain advancement step 4 promised a per-hop
failure detail debug header `X-OLP-Fallback-Detail`. From D9 through
D39 the engine logged per-hop events (D28 added correlation fields)
but never surfaced the failure trail on the response. D40 fulfills
the promise via Option A — ungated v0.1 emission. Phase 2 will
re-introduce owner-vs-non-owner gating when `lib/keys.mjs` lands.

**Per-hop tuple collection** (lib/fallback/engine.mjs)

executeWithFallback now collects `fallbackDetail` — an array of per-hop
tuples — and returns it on EVERY return shape (success / client-error /
AUTH_MISSING / non-trigger / chain-exhausted). Soft-trigger skipped
hops are also recorded with `trigger_type: 'soft'` (currently dead-
code-by-config per ADR 0004 Amendment 2; shape forward-compatible).

Tuple shape (reuses D28 log-event field shapes so logs and the header
pivot on the same keys):
```
{
  hop: <0-indexed hop number>,
  provider: <provider name>,
  model: <model name from IR>,
  code: <ProviderError code, engine-synthetic SOFT_TRIGGER, or 'UNKNOWN'>,
  error_message: <truncated to 200 chars with ellipsis on overflow>,
  trigger_type: 'hard' | 'soft' | 'client_error' | 'auth_missing' | 'unclassified'
}
```

**Header serialiser** (server.mjs)

- `FALLBACK_DETAIL_BYTE_CAP = 4096` (UTF-8 bytes).
- `serializeFallbackDetailHeader(fallbackDetail)` exported. Returns
  `null` for empty/null/undefined → caller omits the header.
- `jsonStringifyAscii` escapes every non-ASCII code point as `\uXXXX`
  to satisfy RFC 7230 §3.2.6 field-vchar (Node's HTTP header validator
  rejects multi-byte UTF-8). The D38 CONCURRENCY_LIMIT synthesised
  message contains a U+2014 em dash — without this escape, every
  CONCURRENCY_LIMIT response crashed at writeHead with
  "Invalid character in header content". The regression test pins
  this exact string.
- 4KB cap algorithm: builds candidates as `[...slice(0, kept), sentinel]`
  and measures the FULL serialised length (including sentinel) before
  comparing against the cap, so the result is guaranteed under cap.
  Tail tuples dropped one at a time. Sentinel form:
  `{ truncated: true, omitted_hops: N }`.

**Header emission** (server.mjs)

- `withFallbackDetailHeader(base, fallbackDetail)` wraps the base
  header object; emits the header only when serialiser returns
  non-null.
- Emitted on chain-exhausted, non-trigger-error, client-error,
  AUTH_MISSING, and success-with-prior-failure paths.
- ABSENT on clean primary success — verified by a dedicated HTTP
  integration test.

**Tests** (test-features.mjs): 452 → 468 (+16):

- Engine-level tuple shape: 2-hop/exhausted, 2-hop/success-with-prior-
  failure, 1-hop/success (empty array), 1-hop/fail, non-ProviderError-
  yields-UNKNOWN, 500-char-message → 200-char-with-ellipsis, client
  error → 1 tuple + client_error trigger type
- Serialiser: empty/null → null, small array round-trip, >4KB cap
  with `{truncated:true,omitted_hops:N}` sentinel, RFC 7230 newline/CR
  escaping, non-ASCII escaping (em dash regression guard for the D38
  CONCURRENCY_LIMIT synthesised message)
- HTTP integration: clean-1-hop-success (header absent), 2-hop-
  exhausted (2 tuples on the wire), 2-hop-success-with-prior-failure
  (1 tuple on the wire)

Pre-commit fold-in (per evidence-first checkpoint #4):

- **Reviewer Suggestion #2**: `jsonStringifyAscii` regex character
  class `[U+0080-U+FFFF]` contains an invisible U+0080 boundary marker
  that editors render as nothing, making the line easy to misread as
  the empty class `[-...]`. Folded in a 5-line comment block above
  the function body explaining the literal byte range and citing
  RFC 7230 §3.2.6.

Two reviewer suggestions not folded:
- **Suggestion #1**: AUTH_MISSING tuple path lacks a dedicated D40
  test. Code is structurally correct (tuple pushed before early-
  return); low priority. Future polish.
- **Suggestion #3**: defensive `err.code != null` guard. Extremely
  low priority — PROVIDER_ERROR_CODES is a closed enum with no
  falsy values.

**ADR amendments** (docs/adr/0004-fallback-engine.md)

- Amendment 5 added at top of amendments stack — full tuple schema,
  cap behavior, RFC 7230 hygiene, ungated v0.1 rationale, Phase 2
  follow-up.
- § Chain advancement step 4 updated to remove TBD / not-yet-
  implemented qualifiers and cross-reference Amendment 5.
- § Observability headers section gains the X-OLP-Fallback-Detail
  schema as IMPLEMENTED at v0.1.

**CHANGELOG.md** — D40 sub-entry appended under existing D38/D39
entries in Unreleased section. No package.json bump per
phase_rolling_mode.

Authority:
- ADR 0004 § Decision § Chain advancement step 4 — D40 fulfils
  the promise
- ADR 0004 Amendment 5 (this commit) — implementation contract
- ADR 0004 § Observability headers — updated to IMPLEMENTED state
- D18 (5 standard X-OLP-* headers) — D40 builds on this convention
- D28 (per-hop structured log fields) — D40 reuses the field shapes
- GitHub issue #7 — closed by this commit
- CC 开发铁律 v1.6 § 10.x — fresh-context opus reviewer independent
- CLAUDE.md release_kit_overlay phase_rolling_mode — no version bump

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus,
independent of drafter): APPROVE. Critical depth checks:
- Tuple pushed once per hop, BEFORE client-error / AUTH_MISSING
  early-return branches — both paths include the failing tuple
- fallbackDetail returned on all 5 return paths (verified line by line)
- Cap algorithm measures FULL serialised length (with sentinel)
  before comparing — no overshoot
- RFC 7230 compliance verified end-to-end: imported the function
  in Node, confirmed em-dash → —, unescaped form throws
  "Invalid character in header content"
- Serialiser handles null / undefined / [] gracefully → header
  absent on clean primary success (HTTP test pins this)
- Hygiene: 0 hits for personal markers, home paths, tokens
- 468/468 tests pass in reviewer's independent npm test run

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
taodeng
2026-05-24 21:44:11 +10:00
co-authored by Claude Opus 4.7
parent bdfea6884b
commit 04f797f917
5 changed files with 683 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+10
View File
@@ -17,6 +17,16 @@ All notable changes to OLP land here. Per `CLAUDE.md` release_kit overlay, this
- **Authority:** ADR 0005 § Cache layer / CacheStore API extension (Part 1); ADR 0004 Amendment 1 (Part 4); GitHub issue #3 — closed by this commit; D16 commit `bafa6d1` non-blocking suggestions — batched here.
- **Test count:** 447 → 452 (3 unit tests for `CacheStore.delete` + 1 log-event integration test + 1 sticky-cache regression test).
### D40 — `X-OLP-Fallback-Detail` header (issue #7)
- **New debug header on responses with a non-empty failure trail** — `lib/fallback/engine.mjs#executeWithFallback` now returns a `fallbackDetail` array of per-hop tuples on every code path. `server.mjs` emits `X-OLP-Fallback-Detail: <JSON-stringified array>` on any response where at least one hop failed before the chain resolved or exhausted (chain-exhausted, non-trigger-error, client-error, AUTH_MISSING, and success-with-prior-failure paths). Header is absent on clean primary success (no failure trail to report).
- **Tuple schema** — `{ hop, provider, model, code, error_message, trigger_type }` per failed hop. `code` is the `ProviderError` code or `'UNKNOWN'` for non-`ProviderError` exceptions; `error_message` is truncated to 200 chars with a U+2026 ellipsis on truncation; `trigger_type` matches D28's `classifyTrigger` output (`'hard'` / `'soft'` / `'auth_missing'` / `'client_error'` / `'non_trigger'`). Field shapes reuse D28's per-hop structured log event keys so logs and the header pivot on the same surface.
- **4KB UTF-8 byte cap** — if the JSON-stringified array exceeds 4096 bytes, tail tuples are dropped and a `{ truncated: true, omitted_hops: N }` sentinel is appended such that the total fits under the cap. Cap calculation uses `Buffer.byteLength('utf8')`, not string length.
- **RFC 7230 hygiene** — non-ASCII code points (e.g. the em dash in the D38 `CONCURRENCY_LIMIT` synthesised error message) are escaped as `\uXXXX` so the header value is pure ASCII. Node's HTTP header validator rejects multi-byte UTF-8 in field values; without this step, em-dash-bearing error messages would crash `res.writeHead`. `JSON.parse` round-trips the escaped form correctly.
- **Gating posture — ungated at v0.1** — the original ADR 0004 § Chain advancement step 4 specified owner-only gating. Per the maintainer decision in issue #7, v0.1 ships the header **ungated** (single-tenant family-scale per ALIGNMENT.md; no PII risk in error details). **Phase 2 will re-introduce owner-vs-non-owner gating when `lib/keys.mjs` lands** — explicit follow-up tracked in AGENTS.md § Key files to know and ADR 0004 Amendment 5.
- **Authority:** ADR 0004 § Decision § Chain advancement step 4 (original promise — D40 fulfils it); ADR 0004 Amendment 5 (D40 ratification); D18 (5 standard X-OLP-* headers; D40 builds on the convention); D28 (per-hop structured log fields; D40 reuses the field shapes); GitHub issue #7 — closed by this commit.
- **Test count:** 452 → 468 (7 engine-level tuple-shape tests + 6 serialiser unit tests including the 4KB cap + non-ASCII regression + 3 HTTP integration tests).
## v0.1.0 — 2026-05-24
### Phase 1 Close — Multi-provider proxy core