refactor: publish gateway-only paper-grounded reasoning contract - #765
refactor: publish gateway-only paper-grounded reasoning contract#765seonghobae wants to merge 65 commits into
Conversation
|
Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (84)
📝 WalkthroughWalkthrough이번 변경은 MLX 직접 전송을 인증된 Changes게이트웨이와 모델 검색
오케스트레이션과 capability 라우팅
정책과 검증
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟡 Moderate · up to The PR changes gateway selection, embedding defaults, provider-control handling, and CLI request validation, but current contracts still allow invalid batch requests, can fail omitted-model embedding calls, may misreport the selected provider, and can forward an orchestrator-only reasoning value to providers; invalid configuration and lint failures can also block or destabilize use. Merge is not ready until these bounded correctness and readiness issues are fixed or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTPServer
participant ModelClient
participant TaskOrchestrator
participant LocalGateway
Client->>HTTPServer: Chat 또는 Responses 요청
HTTPServer->>ModelClient: 모델과 capability 검증
HTTPServer->>TaskOrchestrator: 구조화 출력 또는 이미지 workflow 실행
TaskOrchestrator->>LocalGateway: 인증된 local provider 요청
LocalGateway-->>TaskOrchestrator: 모델 응답
TaskOrchestrator-->>HTTPServer: 계약 검증 결과
HTTPServer-->>Client: API 응답
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@opencode-agent Work on the existing branch at exact head |
|
Closed as unsafe to merge at current head. The PR introduces a self-mutating workflow with contents: write and its exact-head Checks fail: Full unit and contract suite rejects existing tool compatibility contracts with 422, TDD repair fails, and Scorecard reports the workflow permission/credential findings. Reopen only as a focused production patch with direct tests and least-privilege workflow permissions. |
|
@opencode-agent Re-review exact current head SHA: d3b31a2. Review findings were fixed with focused regression coverage; the one-shot self-mutating repair workflow was removed. Please publish a fresh formal verdict only from same-head checks and coverage evidence. |
|
@opencode-agent Review exact current head |
|
@opencode-agent Review exact current head SHA b03187f. The gateway-only patch now fails closed for local:// agents without an explicit KV credential and preserves the 422 tool-surface contract. Use only same-head required Checks and changed-file evidence; publish a fresh formal verdict. |
|
@opencode-agent Re-review exact current head SHA e928ada. The current patch also preserves tool_choice auto/none omit-equivalent behavior while rejecting only unsupported tool execution, and routes discovery tests through the validated transport seam. Use only same-head Checks and changed-file evidence; publish a fresh formal verdict. |
…tpath fix(perf): make provider budget gates constant time
…v-on-765 fix: pass explicit argv to CLI entrypoint
|
Exact-head remediation and verification Head: b6d53af Verification on the identical committed tree:
Every prior hosted check and review is stale for this head. Normal protected merge still requires terminal exact-head checks, structured security evidence, zero unresolved threads, and an independent current-head approval; no bypass or force update. @opencode-agent @cwl-noema-review please perform a review-only evaluation of this exact head. Do not update branches or merge. |
|
Exact-head review disposition for d19e349: the current tree already contains the previously reported fixes for bounded CLI request-read timeout parsing, batch embeddings OpenAPI input/inputs requirements, Responses top_p echo assertion, directly runnable test entrypoints, S106 test-secret annotations, and rejection-contract test names. The current delta from b6d53af only removes two unused production bindings and corrects the Responses test to inspect the final synthesis call; no additional source finding is valid on this delta. Local evidence on this exact tree: full pytest 1729 passed, focused PR suite 131 passed, ruff check passed, compileall passed, git diff --check passed. Hosted required checks remain queued and no qualifying approval exists, so this PR stays WAIT_AND_REMEDIATE; no merge or bypass is performed. |
|
@opencode-agent @cwl-noema-review Please review exact current head d19e349. The exact Azure default-only temperature diagnostic is covered, the same-provider request is retried once without temperature, all threads are resolved, and the full suite passes 1729 tests. Please bind findings and approval to this exact head. |
|
Exact-head local verification update (2026-08-22 KST):\n\n- Current head: |
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headd19e3492192e21e4a040fa3fc13a0793443731bf. -
Head SHA:
d19e3492192e21e4a040fa3fc13a0793443731bf -
Workflow run: 32702015999
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: tests.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (19 files)"]
S2 --> I2["repository behavior"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["required checks"]
Evidence --> S3["Docs (22 files)"]
S3 --> I3["operator or user guidance"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["docs review"]
Evidence --> S4["Test (89 files)"]
S4 --> I4["regression suite"]
I4 --> Conflict["Merge conflict blocks this path"]
Conflict --> V4["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: tests.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (19 files)"]
S2 --> I2["repository behavior"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["required checks"]
Evidence --> S3["Docs (22 files)"]
S3 --> I3["operator or user guidance"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["docs review"]
Evidence --> S4["Test (89 files)"]
S4 --> I4["regression suite"]
I4 --> Conflict["Merge conflict blocks this path"]
Conflict --> V4["targeted test run"]
Merge Conflict Guidance
gh pr checkout 765 --repo ContextualWisdomLab/contextual-orchestrator
git fetch origin main
git merge --no-ff origin/main # or: git rebase origin/main
git status --short
# resolve files, then git add <resolved-files>
# merge path: git commit
# rebase path: git rebase --continue
git push origin HEAD:fix/auto-reasoning-effort-contract-rebased
# rebase path only: git push --force-with-lease origin HEAD:fix/auto-reasoning-effort-contract-rebased |
Resolves 14 conflicted files across the tool-fallback classification system, DNS-pinned discovery transport, and embedding batch registry persistence. Beyond textual conflicts, testing surfaced and fixed three real bugs the merge exposed: - _send_with_retry silently lost main's _is_tool_execution_stopped check during auto-merge (the two branches' rewrites didn't overlap enough to flag a conflict); restored it. - --auto-discover-model-agents collided as two independently-authored CLI flags for different features (bootstrap seed expansion vs. runtime discovery); renamed the seed-expansion one to --auto-discover-seed-agents. - agent_from_discovered/_parse_openai_compatible required chat-model naming eligibility unconditionally, rejecting explicitly-capable non-chat models (e.g. embedding-only sources); relaxed to accept either chat eligibility or a declared capability, while still rejecting malformed/missing model ids. Also: _parse_request_framing's oversized-digit-string handling now matches _request_body_size's classification (413, not 400) via a string-length precheck before int() conversion, avoiding Python's int<->str conversion digit limit; and requirements.lock is regenerated via uv pip compile --universal per this branch's own test_python_lockfile_uses_hash_pinning expectation. Full suite: 1862 passed.
|
Maintainer decision needed before further automation: this 63-commit / ~10.5K-line stack overlaps the same issue-568 reasoning-effort feature that #785 now ships compactly (23 files, ~1.6K lines) and conflicts with current main in 15 files, including the sampling/temperature semantics that landed separately. Proposed resolution: close this as superseded by #785 after confirming no unique commits remain unmerged (the trace-context additions look like the main candidates for salvage as a small follow-up). An automated rebase of the full stack is not safe to merge without line-by-line review of duplicated reasoning-effort behavior. |
|
Integration plan (2026-08-25) — orphaned stack base discovered This branch is the base of a landed-but-orphaned stack: #805, #813, #816, #801 (and related fixes: constant-time budget checks, structured-provider orchestration, budget-stop preservation, telemetry session correlation, request-sampling isolation) were merged into THIS branch, not into main. It must land to bring their content home — do not close it. Merge-state facts
Semantic conflicts requiring decisions (verified by local merge attempt)
Recommended execution order: resolve cost_router+orchestrator first (semantic core), then server/_StateStore, then mechanical theirs=main files (README/AGENTS/docs/workflows), regenerate locks, full suite, update PR body evidence. |
|
Superseded after exact-head decomposition. The only current tip-only repair, preserving operator-declared Bytez endpoint capabilities, is now carried with regression coverage by #834 ( |
Summary
Current-main successor for gateway-owned model selection and paper-grounded reasoning policy. The stale #761 branch is closed unmerged.
local://orhttps://gateway contracts and reject direct MLX transport configuration;temperatureunless the caller explicitly supplies it, then negotiate an explicit provider rejection once without model-name fallback tables;reasoning_effort=autoand other unadvertised reasoning controls off the provider wire;Review repairs
The current exact head closes all current inline findings, including:
uvresolution retainingcolorama,greenlet,tzdata, Python 3.10 compatibility branches, and hashes.The Azure/LiteLLM symptom that triggered this repair no longer receives an orchestrator-invented
temperature=0.2. The exact unsupported-temperature diagnostic triggers one bounded retry of the same provider withouttemperature; provider model-group configuration remains a gateway deployment concern.Sources
The policy cites the literature registered in
docs/papers/README.md, including Route to Reason, Route-and-Reason, Reasoning on a Budget, Ares, Fugu, TRINITY, and Conductor. ADR 0025 additionally records the official uv universal-resolution, PEP 508 marker, and PEP 751 lock-format evidence. Vendor documentation remains the wire-compatibility source, not the model-policy source.Current exact-head verification
Exact head
d19e3492192e21e4a040fa3fc13a0793443731bf:git diff --check: passed;Hosted checks, structured same-head security evidence, and an independent protected approval remain authoritative. No predecessor-head result transfers to this head; no bypass or force update is authorized.
Scope
This PR does not bootstrap provider secrets into the PostgreSQL credential registry. That separate deployment/database slice remains Draft in #764 until this secure discovery and capability boundary is integrated.