Skip to content

[feature] support openrouter ori - #1067

Open
ElegantLin wants to merge 2 commits into
mainfrom
feature/openrouter-ori
Open

[feature] support openrouter ori#1067
ElegantLin wants to merge 2 commits into
mainfrom
feature/openrouter-ori

Conversation

@ElegantLin

@ElegantLin ElegantLin commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a pinned, checksum-verified OpenRouter Ori binary for Linux x64/arm64 and glibc/musl
  • run Ori through a genuine ACP stdio shim that translates its JSONL events into assistant, reasoning, tool, diagnostic, and usage updates
  • preserve Ori native sessions across prompts and expose registry-declared reasoning effort
  • support OpenRouter API-key routing through LiteLLM plus host ori login credentials through the shared native-subscription policy
  • document the --agent ori workflow and reject the unsupported acpx/ori combination

Companion manifest

Verification

  • HOME=<isolated-home> uv run python -m pytest tests/ — 5881 passed, 86 skipped, 7 deselected
  • uv run ty check src/
  • uv run ruff check .
  • uv run ruff format --check src tests
  • clean Ubuntu 24.04 smoke: install pinned Ori, start the shim, complete ACP initialize and session/new
  • companion manifest contract: 22 passed; BenchFlow manifest parity against that checkout: 13 passed

Reference


Devin Review

@ElegantLin
ElegantLin deployed to pypi-internal-preview August 29, 2026 19:17 — with GitHub Actions Active

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

Devin Review

Comment thread src/benchflow/agents/ori.py Outdated
Comment thread src/benchflow/agents/ori.py Outdated
@bingran-you

Copy link
Copy Markdown
Collaborator

Thermo-nuclear code-quality re-review of exact head 4f43f200e35dd69e511a7ca6bde6dddd98003b0e: request changes — not ready to merge.

The focused tests pass, but the implementation introduces structural regressions in canonical ownership, provenance, telemetry contracts, and shared-path branching. Several of those regressions already produce incorrect behavior.

Blocking findings

  1. [P1] Ori bypasses the canonical agent integration layer. registry.py adds procedural install/configuration directly to the already 1,609-line core registry, although BenchFlow's declared source of truth is the manifest system. The manifest-parity check correctly rejects ori as an unmanifested core agent. Either ship Ori behind an ACP shim plus manifest, or deliberately evolve the manifest contract to support session-factory agents. Simply adding another exception would preserve the architectural regression.

  2. [P1] Native JSONL trajectories are mislabeled as trusted ACP provenance. The common commit path assigns trajectory_source="acp" or "partial_acp" to session-factory output (rollout/__init__.py), even though Ori advertises ori-jsonl and never used the ACP transport. Downstream validators use this field as custody/trust information. Add a native/session-factory source, or make the adapter genuinely ACP.

  3. [P1] Native telemetry is an untyped, reflection-based capability with incorrect lifecycle ownership. _collect_native_acp_usage discovers latest_usage_totals and usage_source through getattr, but these are absent from the Session contract. Its checkpoint is reset for the primary connection but not connect_as; a direct two-session probe with cumulative totals 100 then 10 leaves the rollout total at 100 instead of 110. Define a typed native-telemetry capability/snapshot and reset its checkpoint at the single session-binding boundary.

  4. [P1] Token arithmetic violates the result contract. OriSession._record_usage records cache components but omits them from total_tokens. With input 4, output 1, cache-read 6, and cache-write 2, it reports total 5 instead of 13. Put normalization and total calculation in one canonical typed usage model rather than reimplementing it in this adapter.

  5. [P1] The strict JSONL decoder discards real terminal evidence. The pinned Ori binary can emit a plain diagnostic before its terminal JSON result on provider failure. _decode_jsonl raises on that first line, losing the structured failure, session id, and usage that follow. Separate transport diagnostics from typed event decoding and preserve unknown/raw records. The 544-line module should also be decomposed into command execution, decoding, and event translation.

  6. [P2] The OpenRouter subscription-auth branch adds more provider-specific spaghetti to a shared function. uses_native_subscription_auth now duplicates the neighboring Claude gate and has cyclomatic complexity 13. Provider eligibility and native credential markers belong in a typed SubscriptionAuth policy so one generic path handles both providers.

Cleaner target design

The code-judo move is to keep Ori-specific behavior behind the canonical agent boundary, introduce a typed protocol-neutral session telemetry contract, give native trajectories honest provenance, and split JSONL transport from typed event translation. That removes several getattr fallbacks, ACP-name reuse, and provider-specific branches rather than merely rearranging them.

Verification on this SHA:

  • Focused audit suite: 42 passed.
  • Full suite from the earlier exact-head run: 5,882 passed, 86 skipped, 7 deselected.
  • ty, ruff check, and git diff --check passed.
  • test is failing because four changed files are not formatted.
  • manifest-parity is failing because ori is unmanifested.
  • pip-audit is also red for a repository dependency; that appears independent of this Ori change.

No file crosses from below 1,000 lines to above 1,000, but the PR adds 110 lines to the already oversized registry and continues extending the 2,722-line rollout module. Passing tests do not offset the structural and provenance blockers above.

@ElegantLin

Copy link
Copy Markdown
Contributor Author

Addressed all six blockers in b3bad47f:

  1. Canonical integration: Ori now runs behind a real ACP stdio shim. The canonical manifest is in companion PR [feature] support openrouter ori manifest agents#67; its contract suite passes (22 tests), and BenchFlow parity against that checkout passes (13 tests).
  2. Provenance: the old ori-jsonl session-factory adapter is gone. Ori now speaks ACP (initialize, session/new, session/prompt, session/update) so acp / partial_acp accurately describe custody.
  3. Telemetry lifecycle: usage arrives through typed ACP PromptResponse.usage. A shared _bind_agent_connection boundary resets the cumulative checkpoint for both connect() and connect_as(); the 100-token then 10-token regression records 110.
  4. Token arithmetic: typed OriUsage computes total as input + output + cache read + cache write + thought tokens and handles duplicate terminal snapshots.
  5. JSONL evidence: decoding and event translation are split into focused modules. Plain, invalid, and non-object lines become diagnostic thought evidence without preventing later structured result/session/usage records from being consumed.
  6. Subscription routing: native eligibility and credential aliases moved into typed registry policy; the shared router no longer has an Ori/OpenRouter-specific branch.

Also added multi-turn/tool/diagnostic/usage regression coverage and verified the pinned binary + ACP handshake in a clean Ubuntu 24.04 container. Full local validation: 5,881 passed, 86 skipped, 7 deselected; ty, ruff check, ruff format --check, and git diff --check pass.

The current parity check will remain red until companion manifest PR benchflow-ai/agents#67 lands. The pip-audit failure is the pre-existing repository dependency finding noted in the original review.

@ElegantLin
ElegantLin requested a review from bingran-you August 30, 2026 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants