Repository navigation
Conversation
…lter.ts request-log.ts sits a few lines under its file-size threshold and the protocol trace needs room there. filterRequestLogs and filteredRequestLogCount move unchanged, together with the ring capacity they bound tail and limit by, and request-log.ts re-exports them.
…tempt marks Ingresses record which lane they chose, and send sites may record the path an attempt took; the trace is derived once at finalize through path.ts, so the log and the planner use one rule. Marks sit in WeakMaps so RequestLogContext and attempt rows do not grow, and nothing here can throw into the request it describes.
Pins the Responses, native, bridge, combo, explicit-mark and blocked derivations, the no-guess cases that yield no trace, and the DTO limits.
addFinalRequestLog derives the trace from the live context and attempts, and the usage row carries it so the Logs detail survives a restart. Reads re-validate it with parseProtocolTraceV1, so older rows and corrupt ones hydrate with no trace rather than a guessed one.
Lets the Logs page and API ask which requests took a native, translated, legacy-bridge or blocked path, or carry no trace at all. An unknown mode matches nothing instead of being silently ignored, for the reason #2704 gave for model.
…ode filter Pins the finalize-time trace, the usage-row round trip, old and corrupt rows hydrating with no trace, and each protocolMode value including none and an unknown one.
The ingress already decides between the native Chat lane and the Responses bridge; it now records that decision, the rule that declined the native lane, and the request's features, so the trace reports the reason the lane actually used. The decision itself is unchanged.
Caller-forward passthrough is the native Messages lane, the translated path is the bridge, and a disabled surface or a compatibility reject is a refusal before any send. Recording each at the point it is decided lets the Logs trace say which one happened without guessing.
The Logs protocol badge, detail section and filter need labels for the closed mode, hop and disposition vocabulary. The short wire names and the IR acronym stay English in French and Taiwanese Mandarin, as the other protocol names already do, and are allowlisted there.
The badge gives a Logs row a compact client-to-upstream label with the mode spelled out, and the panel lists what the request actually did attempt by attempt. Both validate with parseProtocolTraceV1 and render nothing or "no path data" rather than guessing for old rows.
Adds a protocol path select beside the status filter that selects rows by their final delivery mode, or rows with no path data. The field is optional in the filter state so earlier state shapes keep working.
Wires the badge into the model cell beside the existing row badges and adds the protocol section after the route decision in the detail dialog.
Pins the text-not-colour mode label, the internal hop label, the no-path-data fallback for old and invalid rows, and that the none filter selects exactly the rows the panel reports as having no path data.
Protocol Paths now owns how ingress marks become a persisted trace and the protocolMode filter; the dashboard doc points there and names the new request-log-filter owner.
|
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 |
|
✅ 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: fad3121019
ℹ️ 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".
| } else { | ||
| return undefined; | ||
| } | ||
| const live = (attempts ?? []).filter(attempt => Number.isInteger(attempt.ordinal) && attempt.ordinal > 0); |
There was a problem hiding this comment.
The trace treats every allocated attempt as an observed protocol path, even when sendCount is zero. For example, request-transport.ts allocates an attempt before rejecting unsupported computer_call_output, and native Chat allocates one before request construction can fail with a local 400; both rows will therefore claim an upstream path and include an attempt although no send occurred. Include dispatch evidence such as sendCount > 0 in ProtocolTraceAttempt, and similarly avoid the no-attempt native fallback until native Messages has crossed its send boundary.
Useful? React with 👍 / 👎.
| <label className="muted text-control logs-filter-field"> | ||
| {t("logs.filter.protocol.label")} | ||
| <select className="input select-sm" value={filters.protocolMode ?? "all"} aria-label={t("logs.filter.protocol.label")} onChange={event => onFilterChange({ ...filters, protocolMode: event.target.value as LogProtocolModeFilter })}> | ||
| {LOG_PROTOCOL_MODE_FILTERS.map(mode => <option key={mode} value={mode}>{t(protocolModeFilterKey(mode))}</option>)} |
There was a problem hiding this comment.
This adds a user-visible Logs filter, badge, and detail section, but the commit contains no docs-site/ update and a repository-wide search finds no documentation for protocolMode or the observed protocol path. Add the new Logs workflow and the meaning of the modes/no-data state to the public dashboard documentation, as required for user-facing dashboard changes.
AGENTS.md reference: gui/AGENTS.md:L32-L37
Useful? React with 👍 / 👎.
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. |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed PF-02 at exact head fad31210194eaf5d37b14a474cbafe1ff30842e8, against its PF-01 base. Two layer-specific blockers remain:
src/protocols/trace.ts:223-229treats every allocated attempt as an observed protocol path even whensendCount === 0. Local request-construction/admission failures can therefore claim an upstream path that was never dispatched. Filter attempt traces on actual send evidence and do not synthesize the native no-attempt fallback until the native lane has crossed its send boundary.- This adds a user-visible Logs protocol filter, badge, and detail panel without updating
docs-site/. Document the observed-path workflow, the protocol modes, and the no-trace/no-data state as required by the GUI/source documentation rules.
Please add regressions for zero-send attempts and native pre-dispatch failure, plus the required public documentation/build evidence. PF-02 also remains stacked on PF-01, whose contract extraction is currently changes-requested.
리뷰 · 우선순위 51 / 80이 PR은 한 요청이 어느 길로 나갔는지 로그에 남긴다. 채팅 완성 입구와 Messages 입구는 고른 길을 적는다. Responses는 마지막에 쓴 어댑터를 보고 길을 정한다. 적기가 실패해도 요청은 그대로 끝난다. 끝난 기록은 베이스는 #5808의 라인 - 라인 - 메인테이너의 판단이 필요한 지점 안 보낸 시도를 길에서 뺄지, “보내기 전”으로 따로 보일지 정해 달라. 막힌 요청에 넘긴 기능 목록을 화면에 남길지 정해 달라. 지금은 이 PR은 #5808 다음에 너의 추천 방향은 유지해라. 안 보낸 시도를 길에서 빼고, 대시보드 문서에 필터를 한 줄 넣은 다음 #5808 뒤로 합쳐라. 이 댓글은 grok-bot이 작성했습니다 |
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-02 of the protocol-first-class unit (
devlog/_plan/260924_protocol_first_class/030_gui_and_management_api.md#pf-02-observed-path-trace). Stacked on #5808 (PF-01).Records which protocol path each request actually took, persists it, and shows it in Logs. No request path changes: marks are side-effect-free and a failing mark is dropped, never thrown into the request.
src/protocols/trace.ts: entry, blocked and per-attempt marks held in WeakMaps (soRequestLogContextdoes not grow), andprotocolTraceForRequestderivingProtocolTraceV1at finalize with the shared lane→path rule insrc/protocols/path.ts./v1/chat/completions(native vs bridge, with the native-decline reason) and/v1/messages(caller-forward passthrough, bridge, disabled surface / compatibility reject as blocked). Responses is derived without a mark.src/server/request-log.tswas 4 lines under its size threshold: the/api/logsfilters move byte-for-byte intorequest-log-filter.tsfirst (own commit), thenprotocolTraceis added to the entry, persisted as an optional field of theusage.jsonlrow (validated withparseProtocolTraceV1on read; old rows hydrate without a trace), and filterable withprotocolMode.ProtocolBadgeon Logs rows,ProtocolTracePanelin the detail dialog ("no path data" for rows without a trace; the internal Responses hop is labelled internal), and a protocol-mode filter. Copy added to all 10 locales.Verification
bun x tsc --noEmit(root): exit 0.gui:bun x tsc -b,bun run lint,bun run lint:i18n: exit 0.bun run structure:check,bun run privacy:scan: exit 0.Tests were written (
tests/responses/protocol-trace.test.ts,tests/usage/request-log-protocol-trace.test.ts,gui/tests/logs-protocol-trace.test.tsx) 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).
Logs rows with protocol path badges
Log detail dialog, Protocol path section
Checklist