-
Notifications
You must be signed in to change notification settings - Fork 0
docs(gap-baseline): record SDK-parity (33-37) scoping, reclassify item 32 #1746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
seonghobae
wants to merge
3
commits into
main
Choose a base branch
from
docs/gap-baseline-sdk-parity-scoping
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2628,6 +2628,29 @@ Higgins, S. S., Crepalde, N., & Fernandes, L. (2021). Segmented multiplexity: A | |
|
|
||
| **Evidence / acceptance.** Permanent tests forbid retry/deadline/sampling symbols in the caller and prove one gateway request, one attempt annotation, control-character-safe telemetry, missing-value rejection, valid trailing-comma normalization, and exact changed-line guidance. Fresh exact-head repository checks and reviews remain the admission authority; predecessor-head evidence is not transferable. The remaining runtime work is to preserve distinct `request_too_large`, discovery, rate-limit, provider transport, malformed-output, stale-head, and sandbox-command-timeout categories in hosted logs. | ||
|
|
||
| ## Backlog items 33-37 (OpenAI SDK parity) scoping, and item 32 (batch scope) reclassification — 2026-09-02 | ||
|
|
||
| **Status:** Scoping only, recorded per this session's throttle agreement. Implementation is deliberately deferred — opening new feature PRs right now would add to an already-saturated review queue (2154 queued / 2-4 in-progress GitHub Actions runs at the time of this entry, unchanged or worse than the 60-concurrent-runner-slot ceiling already documented elsewhere in this file) rather than clear it. Resume only once that queue meaningfully eases, or the repository owner explicitly directs otherwise — not on this session's own initiative. | ||
|
|
||
| **Method.** Read `contextual-orchestrator/contextual_orchestrator/server.py` (8744 lines, a raw `http.server`-based OpenAI-compatible gateway, not a framework router) directly from protected `main` (commit `212ff437dc297613289dba2e6064ade9942e07d8`), plus `batch_routing.py` and `file_registry.py`, and grepped for every major OpenAI SDK endpoint family's path string. | ||
|
|
||
| **Already implemented** (broader than expected): `/v1/chat/completions`, `/v1/responses`, `/v1/completions` (legacy, redirects to chat/completions), `/v1/embeddings`, `/v1/models`, `/v1/files`, `/v1/batch/embeddings`, `/v1/videos`, `/v1/images/generations`, `/v1/audio/{speech,transcriptions,generations}`, and `/v1/rerank` (a non-standard extension, not in the OpenAI SDK). | ||
|
|
||
| **Genuinely missing** (item 34 candidates — in the SDK, absent here): | ||
| - `/v1/moderations` | ||
| - `/v1/fine_tuning/jobs` | ||
| - `/v1/audio/translations` | ||
| - `/v1/images/edits`, `/v1/images/variations` | ||
| - `/v1/uploads` | ||
| - `/v1/realtime` (WebSocket, persistent-connection architecture — genuinely different shape from every other endpoint here; deserves its own ADR before any implementation attempt, not a same-pattern addition) | ||
|
|
||
| **Missing but likely low priority:** `/v1/assistants`, `/v1/threads`, `/v1/runs`, `/v1/vector_stores` (the Assistants API). OpenAI itself is sunsetting this family in favor of the Responses API, which this gateway already implements (`/v1/responses`). Building parity for a family the SDK's own vendor is deprecating is in tension with item 34's "if the SDK has it, implement it" principle — recorded as a judgment call to deprioritize, not a decision to never build it. | ||
|
|
||
| **Item 32 (batch endpoint scope) reclassified — corrected per Devin review, item stays open, not "policy already covers it."** `batch_routing.py` does implement a `RoutingPolicy` (`cost_router.py:106`, `self.policy = routing_policy or RoutingPolicy(...)`) that `CostRoutingCoordinator` uses to decide sync-vs-batch for *ordinary* `/v1/chat/completions`-shaped calls. But re-reading `server.py` and `cost_router.py` directly (at `main` commit `212ff437dc297613289dba2e6064ade9942e07d8`) shows this does **not** substantiate the original claim: the explicit `/api/v1/batch_routing_jobs` endpoint (`server.py:7443`) calls `coordinator.submit_batch(...)` (`:7453`) directly — `submit_batch` (`cost_router.py:970`) resolves the request's target model via `_resolve_batch_request` (only for ZDR-tagged requests) and hands off to `self.batch_backend.submit(...)`; neither path consults `self.policy`/`RoutingPolicy` at all, so nothing restricts this endpoint to batch-capable models. Separately, `CostRoutingCoordinator.__init__` (`cost_router.py:117-127`) defaults `self.batch_backend` to `LocalBatchBackend` unless a caller explicitly injects one, and `server.py`'s only constructor call (`server.py:5487`, `CostRoutingCoordinator(orchestrator)`) does not inject one — `PgLlmBatchBackend` (the class that wraps `pg_llm_batch.BatchAPIClient`) is never instantiated anywhere outside `tests/`. So "dispatching batch-eligible requests to pg-llm-batch through an injected BatchAPIClient" does not describe the server's actual default wiring. Item 32 remains open pending real verification of `/api/v1/batch_routing_jobs`'s actual model-resolution and backend wiring — not resolved by `RoutingPolicy`'s existence for a different code path. | ||
|
|
||
| **Item 36 (S3-backed Files API) confirmed as a real gap.** `file_registry.py`'s current `FileOwner`/file-registry logic is a *provider-affinity* registry — it tracks which upstream provider a given gateway file id was proxied to, not a self-hosted store. No S3 (or other object-storage) integration exists. Item 36's "Files API can be self-implemented via S3" is not yet built. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| **Suggested implementation order, if/when resumed** (smallest and most independent first, one PR each, not bundled): `/v1/moderations` → `/v1/audio/translations` (extends the existing audio family) → `/v1/images/{edits,variations}` → item 36's S3-backed Files store (infrastructure-shaped, kept separate) → `/v1/uploads` → `/v1/fine_tuning/jobs` (needs persistent job state, more complex) → `/v1/realtime` last, gated behind its own ADR given the architectural shift to a persistent connection model. Item 22 (Keyverse) was scoped in parallel and found mostly already covered by `ContextualWisdomLab/keyverse#103`'s ADR-accepted draft (ABAC/RBAC PDP + KV + credential store); see that item's own gap-baseline entry for detail. | ||
| ## 2026-09-02 `test_strix_quick_gate.sh` stale cron assertion left broken by the `#1630` cadence lengthening | ||
|
|
||
| **Problem.** The required `exact-head-path-policy` check (which runs `bash | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Info: Batch classification matches pinned source
At the pinned revision,
submit_batchbypassesRoutingPolicy; both production constructors leavebatch_backendunset. Only tests instantiatePgLlmBatchBackend, supporting the open-gap classification.Was this helpful? React with 👍 or 👎 to provide feedback.