mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-21 21:15:10 +00:00
docs(governance): fold in codex round-2 review findings (6 issues)
External Codex CLI review pass 2 surfaced 6 substantive issues that round 1 fold-in missed — the self-consistency trap recurred when fold-in was scoped only to files codex explicitly named in round 1. This commit closes round 2 in full. 1. ADR 0002 contradicted ALIGNMENT.md (P1, codex round 2 finding 1) ADR 0002 still said "three default-enabled (Anthropic, OpenAI Codex, Mistral Vibe)" while ALIGNMENT.md (post round 1) said v0.1 ships zero Enabled Providers. Accepted ADR contradicted constitution. Fix: ADR 0002 + ADR 0001 + docs/adr/README.md index rewritten to Candidate framing. 2. release.yml would publish stale v0.1.0-bootstrap notes (P1, round 2 finding 2) The "Unreleased" amendments would have been silently dropped on tag push because release.yml extracts only the matching version section. Fix: CHANGELOG restructured so the amended state IS the v0.1.0- bootstrap section. Full review history (opus + 2 codex rounds) captured inline. 3. package.json advertised non-existent entrypoints (P2, round 2 finding 3) main/scripts.test/scripts.start pointed to files that do not exist. Local npm test and npm start failed; CI masked. Fix: remove all three from package.json. They return in Phase 1 alongside the real files. test.yml bootstrap-tolerance updated to also skip when scripts.test is absent. 4. models-registry.json missing despite SPOT claim (P2, round 2 finding 4) Fix: minimal stub committed (version + empty providers map). alignment.yml validator now actually runs. 5. alignment.yml commit-citation soft check Bash subshell trap (P2, round 2 finding 5) git log ... while read ... WARN=1 — the while loop ran in a subshell because of the pipe, so WARN never propagated out. The post-loop check always reported "clean" even when warnings fired. Fix: process substitution done less than less than (git log ...). 6. Tier A "permanent" wording inconsistent across ADR 0006 + alignment. yml workflow text (P3, round 2 finding 6) Fix: unified to "Excluded by default with no routine reinstatement path; re-inclusion requires ADR 0006 supersession or amendment with new primary-source evidence." Reviewer: OpenAI Codex CLI (external, fresh-context, pass 2). Iron Rule 10 satisfied — round 2 reviewer was not the implementer of round 1 fold-in. Memory learning updated: the self-consistency trap recurs in the fold-in step. Future fold-ins must grep the entire repo for the concept, not only edit files the reviewer named. See learnings/ai_reviewer_self_ consistency_trap.md in cross-machine memory. Co-Authored-By: Claude Opus 4.7 (noreply@anthropic.com)
This commit is contained in:
co-authored by
Claude Opus 4.7 (noreply@anthropic.com)
parent
91223ee9ab
commit
dff428f3d0
@@ -23,7 +23,7 @@ Found OLP (Open LLM Proxy) as a new project, separate from OCP, with the followi
|
||||
|
||||
**Non-mission (explicit).** Per spec §1, OLP is **not**:
|
||||
- A commercial multi-tenant SaaS. Helicone, OpenRouter, LiteLLM, Portkey, Cloudflare AI Gateway already serve that market with funding, SOC2, dashboards, and team features. OLP enters none of those races.
|
||||
- A generic enterprise AI gateway competing on provider breadth. The provider set is intentionally narrow (currently 3 default + 2 optional tier-1 + 3 optional tier-2) and curated by subscription economics, not by "more is better."
|
||||
- A generic enterprise AI gateway competing on provider breadth. The candidate provider set is intentionally narrow (8 total: 3 anticipated Tier D + 2 anticipated Tier C + 3 anticipated Tier B; v0.1 founding ships 0 Enabled per ALIGNMENT.md § Provider Inventory) and curated by subscription economics, not by "more is better."
|
||||
- A model-capability router. OLP does not auto-route to "the smartest model"; the user picks the model explicitly per chain in `config.routing.chains[]`. Capability routing is a separate problem with separate failure modes and is out of scope for v1.0 and beyond.
|
||||
- A conversation-state store. Memory and continuity are client-side concerns (Memory Continuity, Hermes equivalents, IDE-side context). OLP is a pure stateless proxy.
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
|
||||
## Context
|
||||
|
||||
OLP supports a curated set of providers — three default-enabled (Anthropic, OpenAI Codex, Mistral Vibe) plus two optional tier-1 (xAI Grok, Moonshot Kimi) and three optional tier-2 (MiniMax, Zhipu GLM, Alibaba Qwen). Each provider has its own CLI binary, its own auth artifact location, its own request shape, its own response shape, its own quota-reporting endpoint (or none), and its own rate-limit posture. The maintainer's strong prior is that this set grows over the project's lifetime — provider economics will continue to shift, and "the right five providers" in 2027 will not be identical to today's five.
|
||||
OLP declares a curated set of candidate providers — three anticipated Tier D (Anthropic, OpenAI Codex, Mistral Vibe), two anticipated Tier C (xAI Grok, Moonshot Kimi), and three anticipated Tier B (MiniMax, Zhipu GLM, Alibaba Qwen). Per ALIGNMENT.md § Provider Inventory, all 8 ship as **Candidate** at v0.1 founding; transition to **Enabled** requires authority pin filled + plugin landed + Phase audit passed. Each provider has its own CLI binary, its own auth artifact location, its own request shape, its own response shape, its own quota-reporting endpoint (or none), and its own rate-limit posture. The maintainer's strong prior is that this set grows over the project's lifetime — provider economics will continue to shift, and "the right five providers" in 2027 will not be identical to today's five.
|
||||
|
||||
The naive architecture is a monolithic dispatcher inside `server.mjs`:
|
||||
|
||||
@@ -86,7 +86,7 @@ Every provider plugin exports an object conforming to:
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**(a) Monolithic dispatch inside `server.mjs`.** A single function with `if/else if` per provider, each branch implementing spawn/quota/health inline. Rejected: this is the architectural shape that produced OCP's `server.mjs` length problem at *one* provider, and it does not survive contact with three default + two optional tier-1 + three optional tier-2 providers (eight code paths). Worse, it makes provider-disable a code change, which means fast-quarantine in response to a ToS announcement (spec §9) is a release event rather than a config flip.
|
||||
**(a) Monolithic dispatch inside `server.mjs`.** A single function with `if/else if` per provider, each branch implementing spawn/quota/health inline. Rejected: this is the architectural shape that produced OCP's `server.mjs` length problem at *one* provider, and it does not survive contact with 8 candidate providers (anticipated 3 D + 2 C + 3 B, eight code paths once they all transition from Candidate to Enabled). Worse, it makes provider-disable a code change, which means fast-quarantine in response to a ToS announcement (spec §9) is a release event rather than a config flip.
|
||||
|
||||
**(b) Full external plugin discovery (npm-installable, runtime-loaded, hot-discoverable).** Plugins are npm packages; OLP scans `node_modules/@olp-providers/*` at startup; users `npm install` to add a provider. Rejected for v1.0 on three grounds: (1) the provider set is curated for ToS-risk reasons (ADR 0006), and "anyone can install any provider" defeats that curation; (2) the discovery layer is itself non-trivial code (manifest validation, version compatibility, security review of third-party plugin code) that does not earn its complexity at three to eight providers; (3) the contract has not stabilized enough — locking it as a stable plugin API before v1.0 ships is premature commitment.
|
||||
|
||||
|
||||
@@ -83,7 +83,7 @@ The inclusion ADR is a hard prerequisite for the plugin's merge — no ADR, no m
|
||||
**Negative**
|
||||
- Tier B's consent UX adds a step to first-enable. Users who already understand the risk perceive friction. The friction is intentional; the consent is the structural mechanism that protects the user.
|
||||
- The framework is provider-by-provider, not category-by-category. A new Chinese provider similar to MiniMax/GLM/Qwen gets its own ADR even though the pattern is shared. This is intentional — boilerplate-and-paste ADRs are still cheaper than the alternative of "we just classed them together once and now we have a category drift problem when their ToS shifts."
|
||||
- Tier-A exclusion is *permanent* in the current framework, with no documented reinstatement path. If Google were to walk back the named-tool prohibition and the cost picture changed, Antigravity would need a fresh ADR superseding this one. The framework deliberately makes Tier A "needs a new decision to revisit," not "auto-reconsidered each release."
|
||||
- Tier-A exclusion has **no routine reinstatement path** — re-inclusion requires this ADR to be superseded or amended with new primary-source evidence (per the Tier-A definition above). If Google were to walk back the named-tool prohibition and the cost picture changed, the path is "draft an ADR amendment citing the new primary-source evidence," not a routine config flip. The framework deliberately makes Tier A "needs a constitutional-level decision to revisit," not "auto-reconsidered each release." This is procedurally stricter than "permanent" (the amendment procedure is always available) but practically equivalent — the bar is constitutional, not operational.
|
||||
|
||||
**Mitigations**
|
||||
- The README "Supported Providers" table is the user's first stop and surfaces tiers prominently. The dashboard (spec §4.6) shows tier per enabled provider so users see it on every dashboard load, not only at enable-time.
|
||||
|
||||
+1
-1
@@ -19,7 +19,7 @@ New ADRs increment from the highest existing number. Filenames are `NNNN-<short-
|
||||
| [0003](0003-intermediate-representation.md) | Intermediate Representation (IR) Design | The OLP-internal canonical request/response shape between the OpenAI-compat entry surface and each provider plugin. v1.0 IR fields, IR-vs-OpenAI-vs-native-provider three-shape model, IR is not exposed externally. |
|
||||
| [0004](0004-fallback-engine.md) | Fallback Engine Semantics & Safety | Trigger taxonomy (Hard / Soft / Deterministic-deferred / Cost-aware-deferred), idempotent-failure safety (first-chunk rule), chain advancement one-at-a-time, observability headers. |
|
||||
| [0005](0005-cache-cross-provider.md) | Cache Layer Cross-Provider Design | Cache key composition over `(provider, model, messages, …)`, per-model isolation, D1+D2+D3+D4 port from OCP v3.13.0, cross-provider fallback cache behaviour (correct miss). |
|
||||
| [0006](0006-provider-inclusion.md) | Provider Inclusion / Exclusion + Risk-Tier Framework | The 4-tier classification (A excluded / B explicit consent / C opt-in / D default-enabled), current v0.1 inventory, Antigravity exclusion rationale (named prohibition + no cost advantage + reinstatement friction), consent UX, future provider addition procedure. |
|
||||
| [0006](0006-provider-inclusion.md) | Provider Inclusion / Exclusion + Risk-Tier Framework | The 4-tier classification (A excluded by default / B explicit consent / C opt-in / D eligible-for-default-enabled), Candidate-vs-Enabled distinction, current v0.1 candidate inventory (0 Enabled), Antigravity exclusion rationale (named prohibition + no cost advantage + reinstatement friction; pending primary-source pin), consent UX, future provider addition procedure. |
|
||||
|
||||
## When to write a new ADR
|
||||
|
||||
|
||||
Reference in New Issue
Block a user