Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
📝 WalkthroughWalkthroughAdds Realtime integration tests and test helpers, URL-encodes Realtime WS parameters, and introduces session-oriented logging UI (session sheet, log detail view), log API endpoints/types, message/column rendering for realtime turns, and filter/popover changes to support session-based filtering. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (UI)
participant Sheet as SessionDetailsSheet
participant API as logsApi (REST)
participant WS as WebSocket (log events)
User->>Sheet: Open session sheet (sessionId, liveEnabled)
Sheet->>API: GET /logs/sessions/:id/summary
API-->>Sheet: session summary (count, cost, tokens)
Sheet->>API: GET /logs/sessions/:id (limit, offset, order)
API-->>Sheet: page of logs
alt liveEnabled
Sheet->>WS: subscribe to "log" events
WS-->>Sheet: log event (parent_request_id matches)
Sheet->>Sheet: merge/update sessionLogs, update UI counts
end
User->>Sheet: Click highlighted log
Sheet-->>User: open LogDetailView for that log
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
|
Confidence Score: 4/5Safe to merge; one P2 session.update field mismatch does not block the endpoints under test from functioning correctly All previously flagged P1 issues are resolved. The remaining finding is P2 — the wrong field name in session.update means text-only modality is not enforced, but tests likely still pass because OpenAI defaults to text+audio and emits response.output_text.delta regardless tests/integrations/python/tests/utils/common.py — session.update field name correction Important Files Changed
Reviews (17): Last reviewed commit: "test: add realtime WebSocket and client ..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/integrations/python/tests/utils/common.py (1)
3540-3551: Use a context manager to properly close the OpenAI SDK client.The current code instantiates the client without closing it. Use a context manager or explicitly call
.close()to properly release underlying HTTP resources.Suggested refactor
- client = OpenAI( - api_key=api_key, - base_url=base_url, - timeout=timeout, - default_headers=merged_headers, - ) - - response = client.post( - "v1/realtime/client_secrets", - cast_to=httpx.Response, - body=request_body, - ) + with OpenAI( + api_key=api_key, + base_url=base_url, + timeout=timeout, + default_headers=merged_headers, + ) as client: + response = client.post( + "v1/realtime/client_secrets", + cast_to=httpx.Response, + body=request_body, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integrations/python/tests/utils/common.py` around lines 3540 - 3551, The OpenAI client (instantiated as client = OpenAI(...)) is not closed after use; modify the test to either instantiate the client with a context manager (e.g., using "with OpenAI(...) as client:") or call client.close() after the post call so underlying HTTP resources are released; ensure the post call to "v1/realtime/client_secrets" (client.post) remains inside the context or before the explicit client.close() to avoid using a closed client.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integrations/python/tests/test_openai.py`:
- Around line 4375-4378: Add the missing third compatibility route to the
integration_urls list so the tests exercise the double-prefixed path; update the
list in tests/integrations/python/tests/test_openai.py (where integration_urls,
ws_base and realtime_model are defined) to include a third entry like
f"{ws_base}/openai/openai/realtime?deployment={quote(realtime_model, safe='')}"
(or use ?model=... if your other tests expect model instead of deployment) so
the /openai/openai/realtime route is covered alongside /openai/v1/realtime and
/openai/realtime.
- Around line 4521-4528: The test
test_66_realtime_client_secret_unsupported_provider directly calls
get_api_key("openai") causing failures when OpenAI creds are absent; modify the
test to early-skip when no OpenAI key is available by using the same guard used
in the standalone OpenAI tests (check get_api_key("openai") and call pytest.skip
or return if it is None/empty) so the negative-path assertion for unsupported
providers still runs only in environments with credentials.
In `@tests/integrations/python/tests/utils/common.py`:
- Around line 3407-3416: The loop currently stops after a fixed 10-event cap
which can cause flaky failures; replace the for _ in range(10) loop with a
time-based wait (e.g., compute deadline = time.time() + timeout_seconds, default
~30s) and loop while time.time() < deadline, calling recv_event() until you see
event_type == "session.created" (set got_session_created) or an "error" event
(set error) and break; after the loop, if neither was received, raise/return a
timeout error indicating the handshake timed out. Update references to
recv_event(), got_session_created, and error in
tests/integrations/python/tests/utils/common.py accordingly.
---
Nitpick comments:
In `@tests/integrations/python/tests/utils/common.py`:
- Around line 3540-3551: The OpenAI client (instantiated as client =
OpenAI(...)) is not closed after use; modify the test to either instantiate the
client with a context manager (e.g., using "with OpenAI(...) as client:") or
call client.close() after the post call so underlying HTTP resources are
released; ensure the post call to "v1/realtime/client_secrets" (client.post)
remains inside the context or before the explicit client.close() to avoid using
a closed client.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 66c81d5a-4180-4cdf-aa42-7d0252ae653e
📒 Files selected for processing (2)
tests/integrations/python/tests/test_openai.pytests/integrations/python/tests/utils/common.py
cc0cadd to
ecd583b
Compare
32475da to
8c028c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/integrations/python/tests/test_openai.py (1)
4301-4306: Avoid piggybacking the realtime suite on thesimple_chatscenario matrix.These tests already hard-gate on
OPENAI_REALTIME_MODEL, but the current parametrization adds an unrelated dependency on the standard chat scenario. In a realtime-only setup they collapse to_no_providers_and silently skip even though the realtime model is configured. A small OpenAI/VK-specific parametrization helper would make this suite self-contained.Also applies to: 4347-4352, 4395-4400, 4465-4469
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integrations/python/tests/test_openai.py` around lines 4301 - 4306, The test parametrization is incorrectly reusing get_cross_provider_params_with_vk_for_scenario("simple_chat", include_providers=["openai"]) which ties the realtime tests to the standard chat scenario and causes silent skips; create and use a small OpenAI/VK-specific parametrization helper (e.g., get_openai_vk_params_for_realtime or similar) that returns params independent of the "simple_chat" matrix, update the `@pytest.mark.parametrize` calls that currently reference get_cross_provider_params_with_vk_for_scenario to use this new helper (affecting the parametrize usages around the current test and the other occurrences you noted), and keep the existing OPENAI_REALTIME_MODEL gating logic intact so the suite is self-contained for realtime-only setups.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integrations/python/tests/test_openai.py`:
- Around line 4456-4463: The two assertions for the client_secrets response in
tests/integrations/python/tests/test_openai.py disagree on the JSON shape (one
expects top-level value/expires_at, the other expects nested
client_secret.value/client_secret.expires_at); pick one canonical shape for the
/v1/realtime/client_secrets endpoints and make both assertion sites use the same
structure (update the block that checks result["body"] and the block gated by
response_shape == "sessions" so they assert the identical keys and nesting for
result["body"] and client_secret), ensuring you refer to result["body"],
response_shape, and url when aligning the checks.
---
Nitpick comments:
In `@tests/integrations/python/tests/test_openai.py`:
- Around line 4301-4306: The test parametrization is incorrectly reusing
get_cross_provider_params_with_vk_for_scenario("simple_chat",
include_providers=["openai"]) which ties the realtime tests to the standard chat
scenario and causes silent skips; create and use a small OpenAI/VK-specific
parametrization helper (e.g., get_openai_vk_params_for_realtime or similar) that
returns params independent of the "simple_chat" matrix, update the
`@pytest.mark.parametrize` calls that currently reference
get_cross_provider_params_with_vk_for_scenario to use this new helper (affecting
the parametrize usages around the current test and the other occurrences you
noted), and keep the existing OPENAI_REALTIME_MODEL gating logic intact so the
suite is self-contained for realtime-only setups.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 256b0c8f-5220-4422-97b5-be2d3ea94644
📒 Files selected for processing (2)
tests/integrations/python/tests/test_openai.pytests/integrations/python/tests/utils/common.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integrations/python/tests/utils/common.py
8c028c2 to
88db2f3
Compare
ecd583b to
839db86
Compare
839db86 to
c28e16f
Compare
88db2f3 to
6d5d8d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/integrations/python/tests/utils/common.py (1)
3377-3384: Send the Realtime beta header on WebSocket connects.The HTTP helpers below always send
OpenAI-Beta: realtime=v1, but this helper omits it. That leaves the new WS path under-testing the same compatibility contract this PR is adding for the HTTP realtime helpers.💡 Proposed change
headers = {} if api_key: headers["Authorization"] = f"Bearer {api_key}" if extra_headers: headers.update(extra_headers) + headers.setdefault("OpenAI-Beta", "realtime=v1")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integrations/python/tests/utils/common.py` around lines 3377 - 3384, The WebSocket connect helper currently builds headers (headers/header_list) but doesn't include the Realtime beta header; update the header construction before calling ws_client.create_connection (the code that builds headers, header_list and calls create_connection) to ensure "OpenAI-Beta": "realtime=v1" is set when not already present (merge it into headers or header_list so the ws connect sends the same realtime=v1 beta header as the HTTP helpers).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integrations/python/tests/utils/common.py`:
- Around line 3538-3559: The OpenAI client created in this helper is never
closed, leaking httpx connection pools; wrap the use of OpenAI(...) and
client.post(...) in a try/finally and call the client's close method in the
finally block (e.g., client.close() or await client.aclose() depending on
sync/async usage) so the connection pool is released; update the block around
OpenAI, client.post, response.json handling to ensure client is always closed
even on JSON parsing errors.
---
Nitpick comments:
In `@tests/integrations/python/tests/utils/common.py`:
- Around line 3377-3384: The WebSocket connect helper currently builds headers
(headers/header_list) but doesn't include the Realtime beta header; update the
header construction before calling ws_client.create_connection (the code that
builds headers, header_list and calls create_connection) to ensure
"OpenAI-Beta": "realtime=v1" is set when not already present (merge it into
headers or header_list so the ws connect sends the same realtime=v1 beta header
as the HTTP helpers).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 22f05548-0d2e-40a7-b93c-76cfd48758b1
📒 Files selected for processing (2)
tests/integrations/python/tests/test_openai.pytests/integrations/python/tests/utils/common.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integrations/python/tests/test_openai.py
c28e16f to
bd6ac46
Compare
6d5d8d1 to
99b22d0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integrations/python/tests/utils/common.py`:
- Around line 3500-3503: The default OpenAI-Beta header is set incorrectly;
update the headers dict (variable name "headers") in
tests/integrations/python/tests/utils/common.py so that "OpenAI-Beta" defaults
to None (not "realtime=v1"), and make the code that builds request headers skip
adding the header when its value is None so GA requests do not send the
deprecated header; locate any helper that assembles headers from the "headers"
dict and ensure it omits keys with None values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02e21628-0236-45ff-81f1-313f6df46986
📒 Files selected for processing (2)
tests/integrations/python/tests/test_openai.pytests/integrations/python/tests/utils/common.py
bd6ac46 to
9f9abc4
Compare
99b22d0 to
adc8638
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integrations/python/tests/utils/common.py`:
- Around line 3395-3405: recv_event and run_ws_responses_test check the clock
before calling conn.recv() but don't bound each recv call, so a slow/blocked
recv can repeatedly block up to the socket timeout; fix by computing a real
deadline (deadline = start_time + timeout), then before each conn.recv() compute
remaining = deadline - time.monotonic(), raise TimeoutError if remaining <= 0,
and pass that remaining time into the recv call (or set the socket recv timeout
to remaining) so each recv is limited by the overall deadline rather than the
full socket timeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f98a30d2-b44c-4310-af63-92b4e9bdda8b
📒 Files selected for processing (2)
tests/integrations/python/tests/test_openai.pytests/integrations/python/tests/utils/common.py
9f9abc4 to
e0198ef
Compare
adc8638 to
6861108
Compare
7d8cd01 to
d2b591c
Compare
37d588c to
d30e179
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integrations/python/tests/test_openai.py (1)
4514-4520: Consider assertingexpires_atfor completeness.The test validates
client_secret.valuebut omitsexpires_at, which test_64 checks for the same endpoint type. Adding this assertion would ensure the full response contract is validated.➕ Proposed enhancement
assert result["body"]["client_secret"]["value"], ( f"Missing client_secret.value for base_url={base_url}" ) + assert result["body"]["client_secret"]["expires_at"], ( + f"Missing client_secret.expires_at for base_url={base_url}" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integrations/python/tests/test_openai.py` around lines 4514 - 4520, The test in tests/integrations/python/tests/test_openai.py currently asserts result["body"]["client_secret"]["value"] but omits checking the expiry; update the assertion to also verify result["body"]["client_secret"]["expires_at"] is present (non-empty/null) for the same base_url case so the response contract is fully validated—locate the assertion block using the variables result and base_url and add an analogous assert for client_secret["expires_at"] (or a type check if preferred).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/integrations/python/tests/test_openai.py`:
- Around line 4514-4520: The test in
tests/integrations/python/tests/test_openai.py currently asserts
result["body"]["client_secret"]["value"] but omits checking the expiry; update
the assertion to also verify result["body"]["client_secret"]["expires_at"] is
present (non-empty/null) for the same base_url case so the response contract is
fully validated—locate the assertion block using the variables result and
base_url and add an analogous assert for client_secret["expires_at"] (or a type
check if preferred).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb1368dd-f15a-43dd-8884-f870e4757c4e
📒 Files selected for processing (2)
tests/integrations/python/tests/test_openai.pytests/integrations/python/tests/utils/common.py
d30e179 to
8448b2e
Compare
d2b591c to
589c406
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/integrations/python/tests/test_openai.py (1)
4505-4520:⚠️ Potential issue | 🟠 Major
test_65is asserting the/client_secretsbody as if it were/sessions.
run_openai_base_url_client_secret_request()posts tov1/realtime/client_secrets, but this test still expects a nestedbody["client_secret"]["value"]. In the same file,test_64already treats.../client_secretsas the top-levelvalue/expires_atshape and reserves the nested object for.../sessions, so the SDK compatibility check is validating the wrong contract for the endpoint it actually calls. (raw.githubusercontent.com)🩹 Minimal fix
- assert result["body"]["client_secret"]["value"], ( - f"Missing client_secret.value for base_url={base_url}" - ) + assert result["body"]["value"], ( + f"Missing value for base_url={base_url}" + ) + assert result["body"]["expires_at"], ( + f"Missing expires_at for base_url={base_url}" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integrations/python/tests/test_openai.py` around lines 4505 - 4520, test_65 is validating the wrong response shape for the v1/realtime/client_secrets endpoint: run_openai_base_url_client_secret_request() returns a top-level object (value, expires_at) not a nested client_secret object as used by sessions; update the assertions in test_65 to mirror test_64 by checking result["body"]["value"] and result["body"]["expires_at"] (and adjust the error message text accordingly), rather than asserting result["body"]["client_secret"]["value"].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integrations/python/tests/test_openai.py`:
- Around line 4313-4345: The realtime tests still use the deprecated OpenAI
model name and old event type; update the shared realtime helper so OpenAI's
default realtime model is "gpt-realtime-1.5" (replace "gpt-4o-realtime-preview")
and change any checks for the streaming event "response.text.delta" to
"response.output_text.delta" (this affects the helper used by
get_realtime_test_model / run_ws_realtime_test and any assertions that inspect
event types in the test result). Ensure the extra_headers/vk logic and ws_url
construction remain unchanged and only the default model string and event-type
string are updated wherever they appear.
---
Duplicate comments:
In `@tests/integrations/python/tests/test_openai.py`:
- Around line 4505-4520: test_65 is validating the wrong response shape for the
v1/realtime/client_secrets endpoint: run_openai_base_url_client_secret_request()
returns a top-level object (value, expires_at) not a nested client_secret object
as used by sessions; update the assertions in test_65 to mirror test_64 by
checking result["body"]["value"] and result["body"]["expires_at"] (and adjust
the error message text accordingly), rather than asserting
result["body"]["client_secret"]["value"].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f7c5212-3bbf-4246-bc16-37731f759db4
📒 Files selected for processing (2)
tests/integrations/python/tests/test_openai.pytests/integrations/python/tests/utils/common.py
589c406 to
bab6a32
Compare
8448b2e to
67cde0e
Compare
bab6a32 to
0a7f3ae
Compare
67cde0e to
3cb15fa
Compare
0a7f3ae to
7b591db
Compare
3cb15fa to
7799ba5
Compare
Merge activity
|
The base branch was changed.

Summary
Adds Python integration tests for the Realtime API, covering WebSocket
connections, client secret HTTP endpoints, OpenAI SDK compatibility, and error
handling for unsupported providers.
Changes
run_ws_realtime_testutility: establishes WebSocket connection, runsfull conversation flow (session.created → session.update →
conversation.item.create → response.create → response.text.delta →
response.done), returns structured result
run_realtime_client_secret_requestutility: HTTP POST to client secretendpoints with
OpenAI-Beta: realtime=v1headerrun_openai_base_url_client_secret_requestutility: tests SDKconstructor
base_urloverride compatibilityget_realtime_test_modelhelper: reads provider-specific model from envvars
/v1/realtime/openai/v1/realtime,/openai/realtime)(
/v1/realtime/client_secrets,/v1/realtime/sessions, OpenAI integrationpaths)
base_urlcompatibility for client secret creationType of change
Affected areas
How to test
Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
Checklist
docs/contributing/README.mdand followed the guidelines