From e10b7d7cb9f2e719bfe6fe4feec96a5c9c62c0cd Mon Sep 17 00:00:00 2001 From: dtzp555 Date: Sun, 24 May 2026 14:28:49 +1000 Subject: [PATCH] =?UTF-8?q?docs(adr-0004)+chore:=20D22=20=E2=80=94=20defer?= =?UTF-8?q?=20soft=20triggers=20to=20v1.x=20(round-2=20F2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cold-audit catch from 2026-05-24 Round-2 cold-audit Finding 2 (P2 feature-surface vs data-ingestion drift). ADR 0004 § Trigger taxonomy documented soft triggers (credit_pool_percent, daily_request_count, five_hour_window_percent) as a live, configurable feature category. But `evaluateSoftTriggers` in lib/fallback/engine.mjs is functionally inert at v0.1: - buildDefaultChain hardcodes quotaSnapshot: null on every hop - No call site for provider.quotaStatus() in server.mjs or engine.mjs (only definitions in the 3 provider plugins, all currently stubs) - evaluateSoftTriggers correctly short-circuits to false on null snapshot A user populating routing.soft_triggers in ~/.olp/config.json gets zero runtime behavior. Round-1 cold audit + D5/D9 diff-review reviewers all focused on evaluation correctness; nobody traced the production data path end-to-end. Owner decision (after considering wire-vs-defer): defer to v1.x. Wiring quotaStatus() polling requires per-hop async I/O before each spawn decision, error handling for providers without quota endpoints (all 3 current providers fall in this bucket: claude -p, codex exec --json, vibe --prompt — none expose quota), a caching layer to avoid re-polling, and a latency budget for a pre-spawn network call. Implementation cost high; v0.1 value zero. Strategy: defer the FEATURE (data ingestion path), not the CODE (evaluation logic). evaluateSoftTriggers is small, well-tested via unit tests that inject snapshots directly (test-features.mjs:3617-3665), and architecturally correct. v1.x reactivation requires only wiring the data path — evaluation stays untouched. Changes (3 files, +25 / -1): 1. docs/adr/0004-fallback-engine.md +15 — new Amendment 2 block at top of doc (after Amendment 1, before § Context, matching D11/D15/D16/ Amendment 1 placement convention): - Finding (cold-audit round-2 F2 + 3 concrete code-state facts) - Decision (defer; keep evaluation code inert-but-correct) - Rationale (3-point cost/value analysis) - Effect on § Trigger taxonomy (inline deferral note appended; design prose preserved) - What v1.x reactivation looks like (3 concrete steps; no rewrite needed) - Procedural mechanism (CC 开发铁律 v1.6 § 10.x — round-2 caught it) Plus an inline "📋 Deferred to v1.x (Amendment 2)" sub-bullet in § Trigger taxonomy → Soft triggers entry. The 3 threshold descriptions are preserved verbatim — the architectural design remains the v1.x intent. 2. lib/fallback/engine.mjs +8 / -1 — comment-only updates on the 2 `quotaSnapshot: null` lines in buildDefaultChain. Each now reads: ``` // quotaSnapshot stays null at v0.1 — soft triggers deferred to v1.x per // ADR 0004 Amendment 2. evaluateSoftTriggers correctly short-circuits to // false when quotaSnapshot is null. The polling path is not wired in v0.1. ``` No code logic changed. The previous misleading comment "populated at runtime if provider.quotaStatus() is called" (which falsely implied a live wiring) is replaced. 3. README.md +3 — two additions: - Deferral callout immediately after the routing.soft_triggers config example block, naming Amendment 2 - Implementation status table row: "Soft trigger data path (quotaStatus() polling) | 📋 Planned (v1.x) | Evaluation logic shipped + tested; data ingestion deferred per ADR 0004 Amendment 2" Tests: 335/335 unchanged — pure docs + comment deferral, no behavior change. Existing unit tests for evaluateSoftTriggers (which inject snapshots directly) continue to validate the evaluation correctness independent of the production ingestion path. Authority: - ADR 0004 self-amendment (Amendment 2 in-place) - ALIGNMENT.md Rule 1 (Cite First) + CLAUDE.md § "Hard requirements" item 1 — contract status changes require ADR amendment - CC 开发铁律 v1.6 § 10.x — Round-2 Cold Audit caught this Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent of drafter): APPROVE. Verified all 3 framing claims independently: (a) buildDefaultChain hardcodes null on both branches — confirmed at engine.mjs:421 and :444; (b) zero quotaStatus() call sites in production — `grep -rn "quotaStatus("` found only 3 definitions in provider plugins + JSDoc mentions; (c) evaluateSoftTriggers correctly short-circuits at engine.mjs:139. Reactivation realism check: all 3 v1.x steps map to existing surfaces (insertion points, hop shape, test fixtures). Follow-up items (reviewer's non-blocking notes, NOT in this PR): - README line 154 still reads "328-test suite"; current is 335 — pre- existing doc-sync drift to pick up in D25 P3 batch - v1.x ADR 0002 amendment may need to formally define authContext shape (currently informal in the contract); pre-note as a dependency Co-Authored-By: Claude Opus 4.7 --- README.md | 3 +++ docs/adr/0004-fallback-engine.md | 15 +++++++++++++++ lib/fallback/engine.mjs | 8 +++++++- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 666bb90..3d733e7 100644 --- a/README.md +++ b/README.md @@ -86,6 +86,8 @@ OLP reads its config from `~/.olp/config.json`. The minimum useful shape: } ``` +> **Note:** `routing.soft_triggers` thresholds are parsed and stored but have **no runtime effect at v0.1** — the quota polling path (`quotaStatus()` per hop) is deferred to v1.x per [ADR 0004 Amendment 2](./docs/adr/0004-fallback-engine.md#amendment-2--2026-05-24-soft-triggers-deferred-to-v1x-d22). The evaluation logic exists and is tested; only the production data ingestion path is deferred. + Trigger types, fallback safety, idempotency rules, and the full example config land here when Phase 4 ships. See [ADR 0004 (Fallback Engine Semantics & Safety)](./docs/adr/0004-fallback-engine.md) for the design. --- @@ -147,6 +149,7 @@ Phase 1 is in progress. This table reflects what is currently shipped vs. what i | `lib/cache/keys.mjs` | ✅ Shipped | Content-addressed key computation | | `lib/cache/store.mjs` | ✅ Shipped | In-memory Map (file-backed layout: 📋 Phase 2 storage adapter) | | `lib/fallback/engine.mjs` | ✅ Shipped | Trigger evaluation + chain advancement (ADR 0004) | +| Soft trigger data path (`quotaStatus()` polling) | 📋 Planned (v1.x) | Evaluation logic shipped + tested; data ingestion deferred per ADR 0004 Amendment 2 | | `models-registry.json` | ✅ Shipped | SPOT for `(provider, model)` metadata | | `test-features.mjs` | ✅ Shipped | 328-test suite (CI: `test.yml`) | | `lib/keys.mjs` | 📋 Planned (Phase 2) | Multi-key auth, per-key namespacing, audit log | diff --git a/docs/adr/0004-fallback-engine.md b/docs/adr/0004-fallback-engine.md index a028202..947fb70 100644 --- a/docs/adr/0004-fallback-engine.md +++ b/docs/adr/0004-fallback-engine.md @@ -25,6 +25,20 @@ - **Procedural mechanism:** CC 开发铁律 v1.6 § 10.x (Cold Audit). Diff-review reviewers on earlier passes focused on the first-chunk rule for the real-streaming path; the buffered path has its own truncation-vs-fallback decision point, which the cold-audit pass on 2026-05-23 identified as Finding 17. +### Amendment 2 — 2026-05-24: Soft triggers deferred to v1.x (D22) + +- **Finding:** Cold-audit round-2 Finding 2 (P2 — feature surface implemented but data ingestion not wired). The § Trigger taxonomy in the Decision section below lists soft triggers as a live, configurable feature category. However, `evaluateSoftTriggers` in `lib/fallback/engine.mjs` is inert at v0.1 because: (a) `buildDefaultChain` hardcodes `quotaSnapshot: null` on every hop; (b) no call site for `provider.quotaStatus()` exists in production code paths (`server.mjs` or `lib/fallback/engine.mjs`); (c) `evaluateSoftTriggers` correctly short-circuits to `false` when `quotaSnapshot == null`. A user populating `routing.soft_triggers` in `~/.olp/config.json` gets zero runtime effect at v0.1. + +- **Decision:** Defer soft triggers to v1.x — re-classify them alongside Deterministic-deferred and Cost-aware-deferred in the trigger taxonomy. The evaluation code (`evaluateSoftTriggers` in `lib/fallback/engine.mjs`) stays in place: it is small, well-tested via unit tests that inject quota snapshots directly, and architecturally correct. v1.x reactivation requires only wiring the data ingestion path, not rewriting the evaluation logic. Configured `routing.soft_triggers` thresholds in `~/.olp/config.json` have no runtime effect at v0.1 and are silently ignored. + +- **Rationale:** At v0.1, wiring `quotaStatus()` polling requires: per-hop async I/O before each spawn decision, error handling for providers that do not implement the optional `quotaStatus()` method, a caching layer to avoid re-polling on every request, and a latency budget for a pre-spawn network call. No current provider has a reliable quota endpoint (`claude -p` does not expose quota; `codex exec --json` does not; `vibe --prompt` does not). Implementation cost is high; value at v0.1 is zero. The inert-but-correct state is the conservative call. + +- **Effect on § Trigger taxonomy:** The soft triggers entry in the table below is updated to mark soft triggers as deferred to v1.x, alongside Deterministic-deferred and Cost-aware-deferred. The architectural text describing the soft trigger design (the three threshold types) is retained — it remains the correct v1.x design — with an inline deferral note appended. + +- **What v1.x reactivation looks like:** A single ADR amendment + three code changes: (a) wire `await provider.quotaStatus(authContext)` in `buildDefaultChain` or per-hop in `executeWithFallback`, (b) populate `chainHop.quotaSnapshot` from the polling result, (c) add integration tests verifying the production path produces non-null snapshots that drive evaluation. The `evaluateSoftTriggers` function in `engine.mjs` does not change. + +- **Procedural mechanism:** CC 开发铁律 v1.6 § 10.x (Round-2 Cold Audit). Diff-review reviewers on D5/D9 focused on evaluation correctness; nobody traced the production data path end-to-end to verify `quotaSnapshot` was ever populated. Round-2 cold audit caught it. + ## Context A multi-provider proxy is only valuable if it fails over gracefully. Per spec §1, OLP's core value proposition is "your IDEs and family clients keep working as long as *any* of your subscriptions has quota left." If Anthropic returns 529 (overloaded), or OpenAI returns 429 with `insufficient_quota`, or `claude -p` exits non-zero, the client should not see an error; the client should see the next provider in the chain transparently take over. @@ -56,6 +70,7 @@ Per spec §4.3, the OLP fallback engine implements the following: - `daily_request_count_threshold` — fall back after N requests on the primary today - `five_hour_window_percent_threshold` — fall back based on rolling 5h window quota usage - Soft triggers are evaluated *before* spawn. If a soft trigger fires for the primary, the proxy advances to the next chain entry without attempting the primary at all. + - **📋 Deferred to v1.x (Amendment 2)** — evaluation surface is implemented in `lib/fallback/engine.mjs#evaluateSoftTriggers` and unit-tested, but the production data ingestion path (`quotaStatus()` polling per hop) is deferred to v1.x. Configured `routing.soft_triggers` thresholds in `~/.olp/config.json` have no runtime effect at v0.1. - **Deterministic triggers (deferred to v1.x).** Time-of-day routing, request-content routing ("code requests prefer Codex"). Out of scope for v0.1; tracked in spec §8 future work. diff --git a/lib/fallback/engine.mjs b/lib/fallback/engine.mjs index 0750d64..acb5512 100644 --- a/lib/fallback/engine.mjs +++ b/lib/fallback/engine.mjs @@ -415,7 +415,10 @@ export function buildDefaultChain( provider: hop.provider, model: hop.model ?? modelString, softTriggers: softTriggerConfig[hop.provider] ?? null, - quotaSnapshot: null, // populated at runtime if provider.quotaStatus() is called + // quotaSnapshot stays null at v0.1 — soft triggers deferred to v1.x per + // ADR 0004 Amendment 2. evaluateSoftTriggers correctly short-circuits to + // false when quotaSnapshot is null. The polling path is not wired in v0.1. + quotaSnapshot: null, })); } @@ -435,6 +438,9 @@ export function buildDefaultChain( provider: match.name, model: match.canonicalModel, softTriggers: softTriggerConfig[match.name] ?? null, + // quotaSnapshot stays null at v0.1 — soft triggers deferred to v1.x per + // ADR 0004 Amendment 2. evaluateSoftTriggers correctly short-circuits to + // false when quotaSnapshot is null. The polling path is not wired in v0.1. quotaSnapshot: null, }]; }