Repository navigation
Conversation
planProtocol turns a settled-route snapshot into a ProtocolPlanV1: each candidate's path from path.ts, its feature effects, and whether reject-unrepresentable would refuse it, plus the features every eligible candidate keeps versus only some. It reads no config so the dashboard and the server can share it as a leaf.
The preview needs the route a request would settle on, but routeModel picks combo targets and runs the policy evaluator. Combos and policies are expanded from their configured targets through routeConcreteModel instead, so a preview never advances round-robin state. Messages caller-forward is reported as caller-credential-required because a preview has no caller credential to judge.
GET /api/protocols reports the contract version, surfaces, settings and policy revision; POST /api/protocols/plan returns a preview ProtocolPlanV1. The body is bounded and unknown keys are refused so the route cannot become a place to paste a prompt. Mounted lazily and declared with a deferred-verb exemption owned by PF-12.
Plans are checked with the shared isProtocolPlanV1 validator and cached per target, selector, sorted features and policy revision, so a cached preview is reused only while the policy that produced it is still active. A 404 from an older server reads as unavailable rather than as an error.
A "Request path preview" section after the endpoints lets an operator pick a model, client API and features and see each candidate's path, delivery mode, fidelity, feature effects and reasons, split into features every candidate guarantees and those only some keep. Delivery mode is labelled as delivery, never as verification.
Structure docs name the planner's inputs, why the snapshot expands combos and policies itself, and where the dashboard panel and its validation live; the public management API reference lists the two read-only routes and states that a preview sends nothing.
…exists Toward a different wire the path reason already explains the route; repeating the ingress's cross-wire decline next to it read as a second, contradictory cause.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f7f9cedcc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| export function planProtocol(input: ProtocolPlanInput): ProtocolPlanV1 { | ||
| const features = orderedFeatures(input.features); | ||
| const snapshotCandidates = input.candidates.slice(0, PROTOCOL_DTO_LIMITS.candidates); |
There was a problem hiding this comment.
For a valid combo or routing profile with more than 16 targets—the existing validators impose no such maximum—this slice silently removes every later target before computing candidates, guaranteedFeatures, and partialFeatures. If an omitted target uses a different adapter or loses a requested feature, the preview can falsely report that all candidates preserve it. Either reject configurations above the DTO limit, represent truncation explicitly, or compute the aggregate over every target while bounding only the serialized detail.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| const key = protocolPlanCacheKey(apiBase, query, info.policyRevision); | ||
| const cached = planCache.get(key); | ||
| if (cached) return { kind: "plan", plan: cached }; |
There was a problem hiding this comment.
After a live provider, default route, combo, or routing-profile edit, this lookup can return a stale plan without issuing the POST: policyRevision only hashes apiSurfaces, claudeCode, and protocols, even though planning also reads providers, aliases, combos, profiles, and the default provider. Include those routing inputs or a config generation in the revision/cache key, or remove this cache so the dashboard stays aligned with the active provider configuration.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
| } else if (result.kind === "error") { | ||
| setFailed(true); | ||
| } else { | ||
| setPlan(result.plan); |
There was a problem hiding this comment.
If a preview has already succeeded and the user changes the model, inbound API, or features, a failed subsequent request sets the error flag but retains the previous plan. The panel then renders the failure message together with an obsolete result beneath the newly selected controls, which can be mistaken for the requested preview. Clear the prior plan when starting a new request or when this error branch is reached.
Useful? React with 👍 / 👎.
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed PF-03 at exact head 4f7f9cedcc3e71cff09f4a05e4fdd5bdf93c82de, against its PF-02 base. Three layer-specific blockers remain:
src/protocols/plan.ts:140truncates candidates to the 16-row DTO limit before calculating eligibility and guaranteed/partial features. A valid combo/profile with more targets can therefore hide a later incompatible route and falsely claim full preservation. Aggregate over every candidate and bound only serialized detail, reject oversized configurations, or represent truncation explicitly.gui/src/protocol-api.ts:93-95caches by apolicyRevisionthat excludes provider/default route/alias/combo/profile changes even though planning reads them. A live routing edit can return an obsolete plan without sending the preview POST. Include every routing input (or a config generation) in the revision/key, or remove the cache.ProtocolPlanPanel.tsx:182-199retains the previous successful plan when the next preview fails, so stale output is rendered beneath the new controls together with the error. Clear the plan at request start or on the error branch.
Please add coverage for >16-target aggregation/truncation semantics, routing edits invalidating cached previews, and successful-preview followed by failed-preview state. PF-03 remains a draft stacked on changes-requested PF-01/PF-02 revisions.
리뷰 · 우선순위 54 / 80이 PR은 요청을 보내지 않고, 그 모델이 어느 길로 갈지 미리 보여 준다. API 키 화면에 "요청 경로 미리보기"가 생긴다. 모델, 클라이언트 API, 기능을 고르고 Preview를 누르면 서버가 후보마다 가는 길, 배달 방식, 기능이 살아남는지를 계산한다. 계산은 설정만 읽는다. 업스트림으로는 아무것도 나가지 않고, 콤보의 차례도 그대로다. 계산은 두 겹이다. 화면은 라인 - 라인 - 라인 - 라인 - 라인 - 라인 - CI 메인테이너의 판단이 필요한 지점
16개 상한을 미리보기 전체에 적용할지, 집계는 전부 하고 화면에 나가는 후보만 16개로 할지 정해 달라. 캐시는 라우팅 설정까지 키에 넣을지, 이 화면에서는 캐시를 뺄지 정해 달라. CLI 동사는 PF-12까지 미룰 수 있다. 그때까지 패리티 검사가 빨갛다. 그 검사의 목록에 두 경로를 PF-12 소유로 넣을지, 지금 동사 껍데기를 넣을지 정해 달라. 너의 추천 방향은 유지해라. 닫을 중복 PR은 없다. 베이스를 이 댓글은 grok-bot이 작성했습니다 |
The French catalog test rejects English values that carry translatable words; the route label and its combo value were left in English.
A candidate is identified by its provider and model; the list index added nothing and React Doctor flags index keys.
The API page layout test forbids the classic viewMode toggle by searching for the substring, which the previous prop name contained.
The protocol routes have owed CLI verbs, recorded as deferred in the route registry; the parity map now says so instead of leaving them uncovered.
devin-ai-integration
Bot
added
the
priority: P3
|
Maintainer triage: Criteria (P3): Low: new provider/client integration, large or experimental feature (>2000 LOC or >50 files), RFC/roadmap, or long-stale branch. Related / overlapping PRs:
|
2 participants
Summary
PF-03 of the protocol-first-class unit (
devlog/_plan/260924_protocol_first_class/030_gui_and_management_api.md#pf-03-planner-and-preview). Stacked on #5809 (PF-02).A read-only request-path preview: which wire each candidate of a model would receive for a given client API and feature set, computed with the same lane→path rule the observed trace uses. Nothing is sent, refreshed, selected or written.
src/protocols/plan.ts(pure leaf):planProtocolcomputes each candidate's request/response path, feature effects, eligibility under the unrepresentable policy (feature-unrepresentableunderreject), and the guaranteed-by-all vs some-candidates-only feature split. A disabled surface blocks candidates withsurface-disabled.src/protocols/plan-snapshot.ts: builds the planner input from config without side effects.routeModeladvances combo round-robin state and runs the policy evaluator, so combo and policy selectors are expanded from their configured targets throughrouteConcreteModelinstead; a test pins that a preview leaves combo selection state unchanged. Messages caller-forward is reported ascaller-credential-required, never assumed.GET /api/protocolsandPOST /api/protocols/planinsrc/server/management/protocol-routes.ts, lazily mounted, declared in the route registry with a deferred-verb exemption (CLI verbs land with PF-12). Input is bounded (model ≤ 200 chars, ≤ 24 features, unknown keys → 400) and never logged.structure/owner docs and the management API reference.Verification
bun x tsc --noEmit(root): exit 0. New test files additionally typechecked with a temporary tsconfig: exit 0.gui:bun x tsc -b,bun run lint:i18n: exit 0.bun run structure:check,bun run privacy:scan: exit 0.Tests (
tests/responses/protocol-plan*.test.ts,tests/server/protocol-routes.test.ts,gui/tests/protocol-api.test.ts) were written and registered but run in the full local suite below.Full local run on the stack head (feat(protocols): protocol paths as a first-class concern — PF-01..PF-12 #5820, which contains this change):
bun run test— the only failures are Lab CL-03/CL-07/CL-08/SEC-02 andrelease helpertimeouts, which fail identically on a checkout without this stack (local environment), plus service/toggle cases that pass when run alone;cd gui && bun test --isolate tests— 2398 pass, 0 fail.CI on this head: all required checks pass.
Screenshots
Captured from the stack head in an isolated home (fake providers, no real credentials).
API page, Request path preview for a mixed combo
Checklist