mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-21 21:15:10 +00:00
fix(ir): accept OpenAI role=developer at entry surface, normalize to system (#65)
* fix(ir): accept OpenAI role=developer at entry surface, normalize to system Hermes Agent (v0.13+) and other modern openai-completions clients (Cline, Continue.dev) default to role=developer for the high-priority instruction slot when the model id matches OpenAI o1/o3+ reasoning family. OLP IR's validator rejected this with 400 IR validation failed: role must be one of system|user|assistant|tool, got "developer". Reproduced 2026-05-27 on PI230 Hermes v0.14.0 trying olp-codex/gpt-5.5 through PI231 OLP. olp-claude (Anthropic models) was unaffected because Hermes sends system role for Claude models (no developer-role concept on Anthropic side). Fix: extend openai-to-ir.mjs normalizeRole to map developer to system at the entry boundary. The IR canonical-four-roles invariant is preserved; every provider plugin's role-handling stays unchanged. Same pattern as the existing function-to-tool normalization that already lives in normalizeRole (function role was OpenAI-deprecated). Normalize-at- entry centralizes role-spec-evolution handling in one file rather than bloating the IR schema and forcing every provider plugin to handle each new role. Why not add developer to VALID_ROLES: it would require branches in three provider plugins (anthropic, codex, mistral) all mapping to system-style annotation anyway, plus wider IR surface for any future OpenAI role addition. Tests: two new pin tests in Suite IR translation: - developer role to system translation - mixed-role array including developer validates cleanly 768 to 770 tests, 0 fail. Verified Hermes via olp-codex no longer hits the IR rejection error after this fix (smoke run from PI230). ADR 0003 Amendment 3 documents the rationale + future-role policy (normalize-at-entry unless a role genuinely conveys provider-distinguishable semantics). Authority: OpenAI Responses API spec developer-role + Hermes Agent v0.14.0 reproduction. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test+docs: PR #65 fold-in reviewer N2 + N3 N2 — Negative control test added: unknown role (e.g. "admin") still raises BadRequestError. Pins that the developer-to-system normalization is the only entry-surface escape hatch; any future widening of normalizeRole that returns role as-is for unknown inputs will be caught by this test. N3 — ADR 0003 Amendment 3 gains a "Cache-key impact" bullet documenting that a developer-form and system-form request with otherwise-identical content now share the same IR and thus the same cache key (per ADR 0005). This is by design and matches OpenAI's own backward-compat semantics; the note exists so a future debug session investigating "why does my developer request hit a cache entry from an old system request" finds the answer. Test count: 770 to 771, all pass. Skipped reviewer N1 (URL precision) — Amendment already cites multiple authorities including the Hermes/Cline tracking convention and live reproduction transcript; single-URL precision is over-spec. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: dtzp555 <dtzp555@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,23 @@
|
|||||||
|
|
||||||
## Amendments
|
## Amendments
|
||||||
|
|
||||||
|
### Amendment 3 — 2026-05-27: Accept OpenAI `role: "developer"` at entry surface, normalize to `system` in IR
|
||||||
|
|
||||||
|
- **Finding:** Hermes Agent v0.13/v0.14 (and likely Cline, Continue.dev, and other modern openai-completions clients) default to `role: "developer"` for what was historically the `system`-role slot when the model id matches OpenAI's o1/o3+ reasoning family. The `developer` role was introduced by OpenAI's Responses-API spec for reasoning models (high-priority developer-authored instructions; semantically a peer of `system`). OLP IR's role allow-list at v0.1 was the original four roles (`system|user|assistant|tool`); the IR validator rejected `developer` with `400 IR validation failed: role must be one of system|user|assistant|tool, got "developer"`. Reproduced 2026-05-27 on PI230 Hermes v0.14.0 → OLP v0.5.1 routing path.
|
||||||
|
- **Decision:** Extend `openai-to-ir.mjs:normalizeRole()` to map `developer` → `system` at the entry boundary. The IR's canonical-four-roles invariant is preserved; every provider plugin's role-handling stays unchanged. The normalize-at-entry pattern matches the existing `function` → `tool` normalization that already lives in the same function (function-role-deprecation was the precedent for entry-boundary normalization vs. IR schema bloat).
|
||||||
|
- **Why not "add `developer` to VALID_ROLES + handle in every provider":** That alternative would require:
|
||||||
|
- Expanding `VALID_ROLES` in `lib/ir/types.mjs`.
|
||||||
|
- Adding `developer` branch in `anthropic.mjs:irToAnthropic` (which would map to `[System]` annotation anyway).
|
||||||
|
- Adding `developer` branch in `codex.mjs:irToCodex` (would map to `[System]` annotation anyway).
|
||||||
|
- Adding `developer` branch in `mistral.mjs:irToMistral` (same).
|
||||||
|
- Coordinating every future role addition (e.g., if OpenAI adds another role tomorrow) across N provider plugins.
|
||||||
|
- Wider IR surface area = more drift-prone over time.
|
||||||
|
Normalize-at-entry centralizes role-spec-evolution handling in one file. ADR 0003's IR-design principle ("encode the common subset every provider plugin can consume") supports keeping the IR minimal.
|
||||||
|
- **Forward note:** Future OpenAI role additions follow the same pattern: extend `normalizeRole()`. If a role genuinely conveys provider-distinguishable semantics (e.g., a hypothetical role that meaningfully changes anthropic vs codex behavior), the calculus flips and a IR-level addition would be justified. That decision goes through a new ADR 0003 amendment.
|
||||||
|
- **Cache-key impact:** After this amendment, a request whose first message uses `role: "developer"` and one using `role: "system"` with otherwise-identical content produce the **same** IR (because normalization happens before IR construction) → the **same** cache key (per ADR 0005 cache key composition). This is intentional and matches OpenAI's own backward-compat behavior ("system message with reasoning models is treated as developer"). If a future debug session is investigating "why does my new `developer` request hit a cache entry from an old `system` request" — this is by design.
|
||||||
|
- **Tests:** Suite IR translation in `test-features.mjs` gains three pin tests: (a) `role: "developer"` → `role: "system"` translation, (b) mixed-role array including developer validates cleanly through to IR, (c) negative control — an unknown role (e.g. `"admin"`) still raises `BadRequestError`, confirming the normalize-at-entry mapping did not accidentally widen the role allow-list.
|
||||||
|
- **Authority:** OpenAI Responses API spec — developer role documented as high-priority developer-authored instructions for o1/o3+ reasoning models (https://platform.openai.com/docs/api-reference/responses). Hermes Agent / Cline / Continue.dev tracking the same convention. Reproduced live on PI230 → PI231 OLP 2026-05-27.
|
||||||
|
|
||||||
### Amendment 2 — 2026-05-24: Correct model-mapping example; document verbatim-pass-through design (D32 F2)
|
### Amendment 2 — 2026-05-24: Correct model-mapping example; document verbatim-pass-through design (D32 F2)
|
||||||
|
|
||||||
- **Finding:** Round-4 cold-audit F2 (P3 ADR example vs implementation drift) — § Decision "Required fields" item `model` reads: "The provider plugin maps this to the provider-native model identifier (e.g., `claude-sonnet-4-6` → `claude-sonnet-4-6-20260301` for Anthropic)." This is WRONG per the D17 SPOT decision (commit `cb86807`): OLP does NOT perform a model-alias mapping inside the provider plugin. `irRequest.model` is passed verbatim to the provider CLI (`claude -p --model <model>`, `codex exec --model <model>`, etc.); each provider's CLI resolves its own aliases natively per its documented behaviour.
|
- **Finding:** Round-4 cold-audit F2 (P3 ADR example vs implementation drift) — § Decision "Required fields" item `model` reads: "The provider plugin maps this to the provider-native model identifier (e.g., `claude-sonnet-4-6` → `claude-sonnet-4-6-20260301` for Anthropic)." This is WRONG per the D17 SPOT decision (commit `cb86807`): OLP does NOT perform a model-alias mapping inside the provider plugin. `irRequest.model` is passed verbatim to the provider CLI (`claude -p --model <model>`, `codex exec --model <model>`, etc.); each provider's CLI resolves its own aliases natively per its documented behaviour.
|
||||||
|
|||||||
+17
-2
@@ -27,13 +27,28 @@ export class BadRequestError extends Error {
|
|||||||
// ── Role normalization ────────────────────────────────────────────────────
|
// ── Role normalization ────────────────────────────────────────────────────
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* OpenAI deprecated role='function' in favour of role='tool'.
|
* Normalize entry-surface role names → IR canonical set (system/user/assistant/tool).
|
||||||
* Per ADR 0003, IR supports system/user/assistant/tool.
|
*
|
||||||
|
* Per ADR 0003, IR supports exactly four roles. OpenAI's chat-completions
|
||||||
|
* spec has evolved beyond that, and we keep the IR minimal by normalizing
|
||||||
|
* at the entry boundary instead of bloating IR + every provider plugin.
|
||||||
|
*
|
||||||
|
* Current normalizations:
|
||||||
|
* - `function` → `tool` — deprecated in OpenAI chat API, replaced by tool.
|
||||||
|
* - `developer` → `system` — OpenAI o1/o3+ reasoning models accept a new
|
||||||
|
* "developer" role with similar semantics to "system" (high-priority
|
||||||
|
* instructions from the developer to the model). Providers like Hermes
|
||||||
|
* Agent and Cline default to `developer` for openai-completions calls.
|
||||||
|
* OLP-side anthropic + codex providers don't differentiate developer
|
||||||
|
* from system, so the IR canonicalizes to `system` and downstream
|
||||||
|
* translations remain unchanged.
|
||||||
|
*
|
||||||
* @param {string} role
|
* @param {string} role
|
||||||
* @returns {string}
|
* @returns {string}
|
||||||
*/
|
*/
|
||||||
function normalizeRole(role) {
|
function normalizeRole(role) {
|
||||||
if (role === 'function') return 'tool';
|
if (role === 'function') return 'tool';
|
||||||
|
if (role === 'developer') return 'system';
|
||||||
return role;
|
return role;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -319,6 +319,45 @@ describe('openAIToIR translation', () => {
|
|||||||
assert.equal(ir.messages[0].name, 'my_fn');
|
assert.equal(ir.messages[0].name, 'my_fn');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('maps role=developer to role=system (OpenAI o1/o3+ reasoning shape)', () => {
|
||||||
|
const ir = openAIToIR({
|
||||||
|
model: 'm',
|
||||||
|
messages: [{ role: 'developer', content: 'High-priority instructions.' }],
|
||||||
|
});
|
||||||
|
assert.equal(ir.messages[0].role, 'system', 'developer should normalize to system');
|
||||||
|
assert.equal(ir.messages[0].content, 'High-priority instructions.');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('mixed roles including developer all validate cleanly through IR', () => {
|
||||||
|
const ir = openAIToIR({
|
||||||
|
model: 'm',
|
||||||
|
messages: [
|
||||||
|
{ role: 'developer', content: 'Be terse.' },
|
||||||
|
{ role: 'user', content: 'Hi.' },
|
||||||
|
{ role: 'assistant', content: 'Hello.' },
|
||||||
|
],
|
||||||
|
});
|
||||||
|
assert.equal(ir.messages.length, 3);
|
||||||
|
assert.equal(ir.messages[0].role, 'system');
|
||||||
|
assert.equal(ir.messages[1].role, 'user');
|
||||||
|
assert.equal(ir.messages[2].role, 'assistant');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects an unknown role at entry — normalize didn\'t accidentally widen allow-list', () => {
|
||||||
|
// Negative control for Amendment 3 — without this pin, a future addition
|
||||||
|
// to normalizeRole that returns `role` as-is for unknown inputs would silently
|
||||||
|
// widen the IR role allow-list. Confirms developer→system mapping is the only
|
||||||
|
// entry-surface escape hatch.
|
||||||
|
assert.throws(
|
||||||
|
() => openAIToIR({
|
||||||
|
model: 'm',
|
||||||
|
messages: [{ role: 'admin', content: 'I am god.' }],
|
||||||
|
}),
|
||||||
|
err => err instanceof BadRequestError && /role must be one of/.test(err.message),
|
||||||
|
'unknown role should still produce BadRequestError'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it('translates request with tools', () => {
|
it('translates request with tools', () => {
|
||||||
const ir = openAIToIR({
|
const ir = openAIToIR({
|
||||||
model: 'm',
|
model: 'm',
|
||||||
|
|||||||
Reference in New Issue
Block a user