Add Codex-backed dataset generation client - #47
Merged
Conversation
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 6 minutes and 50 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThis PR introduces dual LLM client support by adding Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
kdunee
force-pushed
the
dataset-generation-improvements
branch
from
May 24, 2026 17:47
6d8726a to
3554f57
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_openai_codex_client.py (1)
1-451:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Ruff formatting to unblock CI.
ruff format --checkis currently failing for this file in pipeline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_openai_codex_client.py` around lines 1 - 451, Run the code formatter (ruff format) on this test file to fix import ordering, trailing commas, line breaks and other style issues that are failing CI; specifically reformat the module containing _sse, _write_auth, _FakeCodexClient and OpenAICodexTextClientTests so imports are grouped/sorted, long lines are wrapped, and any minor typing/import adjustments are applied, then commit the formatted file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ai_research/dataset_generation/cli.py`:
- Around line 26-63: PROMPT_ARTIFACT_MARKERS currently omits "[Explanation]" and
matching is case-sensitive; update PROMPT_ARTIFACT_MARKERS to include
"[Explanation]" and change _contains_prompt_artifact to perform case-insensitive
matching (e.g., compare value.lower() against a pre-normalized list of lowercase
markers or lower each marker at runtime) while keeping existing dict/list
recursion in _contains_prompt_artifact to ensure all nested strings are checked.
In `@ai_research/dataset_generation/infrastructure/openai_client.py`:
- Around line 73-76: The chat completion call in openai_client.py currently only
passes model and messages; update the call in the method that uses
self._client.chat.completions.create to forward generation args from the client
(e.g., self.temperature, self.max_completion_tokens, self.max_tokens) and
conditionalize them by model: for "gpt-4o-mini" include temperature (if set) and
max_completion_tokens; for "gpt-5-mini" do not pass temperature or legacy
max_tokens but do pass max_completion_tokens when present; ensure you only pass
parameters supported by openai==1.109.1 to avoid incompatible fields.
In `@ai_research/dataset_generation/infrastructure/openai_codex_client.py`:
- Around line 46-55: Add a configurable HTTP timeout parameter to the class (via
__init__) and use it when calling urlopen in both _post_stream and _post_json to
avoid indefinite hangs; ensure infer closes the urlopen response (wrap the
response/stream from _post_stream/_post_json in a try/finally or context manager
and call close() after _parse_sse completes) to prevent socket/FD leakage; and
update _refresh_credentials so it only writes auth_payload["last_refresh"] (and
updates stored access token) when the refresh response returns a non-empty
access_token (validate presence and non-empty string before persisting), leaving
the old token/last_refresh untouched if refresh failed.
- Around line 213-221: The _refresh_credentials logic unconditionally sets
auth_payload["last_refresh"]; change it so last_refresh is only updated when a
new access_token was actually returned and stored: after iterating fields into
tokens (the loop over "id_token","access_token","refresh_token" and the
account_id handling), detect whether response provided a non-empty access_token
(or tokens["access_token"] changed) and only then set
auth_payload["last_refresh"] = datetime.now(timezone.utc).isoformat(); leave
auth_payload untouched when access_token was not refreshed.
In `@tests/test_dataset_generation.py`:
- Around line 458-567: The failing tests use local fakes/mocks
(_FakeCompletions, _FakeOpenAI, patch) instead of exercising the real
IntentGuard/OpenAI integration; replace the mocked unit approach in
test_openai_client_sends_prompt_without_token_limit (and any tests referencing
_FakeCompletions/_FakeOpenAI or patching logging/basic config for behavior) with
an integration-style test that uses the real IntentGuard/OpenAI client or the
provided integration test fixture: remove creation/assignment of client._client
= _FakeOpenAI and assertions about internal calls (completions.calls), instead
instantiate OpenAITextClient normally (or via the intentguard test fixture),
perform client.infer("Hello {{.Name}}", {"Name":"world"}) and assert the
end-to-end response and any observable request behavior via the integration
fixture (e.g., recorded request metadata) rather than inspecting private fake
internals; delete or refactor the _FakeCompletions/_FakeOpenAI helpers and
update test assertions to verify observable outputs from the real API/fixture.
In `@tests/test_openai_codex_client.py`:
- Around line 106-450: This test file contains unit-style tests (class
OpenAICodexTextClientTests) that use _FakeCodexClient and call private helpers
like OpenAICodexTextClient._parse_sse; change them to integration-style tests
that exercise the real IntentGuard API and public client surface: remove or
rewrite tests that use _FakeCodexClient, _sse, or call private methods (e.g.,
_parse_sse) and instead instantiate the real client via the public factory
(_build_client or OpenAITextClient/OpenAICodexTextClient with real credentials),
perform live infer calls against the IntentGuard endpoint, assert end-to-end
behavior (HTTP headers, streaming output, retries, token refresh) using real
responses, and update setup/teardown to rely on real CODEX_HOME/credentials
rather than writing fake auth files. Ensure no tests import or assert internal
private helpers or fakes and that all assertions target public behavior only.
---
Outside diff comments:
In `@tests/test_openai_codex_client.py`:
- Around line 1-451: Run the code formatter (ruff format) on this test file to
fix import ordering, trailing commas, line breaks and other style issues that
are failing CI; specifically reformat the module containing _sse, _write_auth,
_FakeCodexClient and OpenAICodexTextClientTests so imports are grouped/sorted,
long lines are wrapped, and any minor typing/import adjustments are applied,
then commit the formatted file.
🪄 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: 8fc9482f-b77a-4be9-bda6-50688fd85e5a
📒 Files selected for processing (5)
ai_research/dataset_generation/cli.pyai_research/dataset_generation/infrastructure/openai_client.pyai_research/dataset_generation/infrastructure/openai_codex_client.pytests/test_dataset_generation.pytests/test_openai_codex_client.py
Comment on lines
+73
to
+76
| response = self._client.chat.completions.create( | ||
| model=self.model, | ||
| messages=messages, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For openai==1.109.1, what are the currently supported chat.completions.createparameters forgpt-4o-miniandgpt-5-mini(specificallytemperature, max_tokens, and max_completion_tokens)?
💡 Result:
For openai==1.109.1, the currently supported chat.completions.create parameters for gpt-4o-mini and gpt-5-mini are: gpt-4o-mini (chat.completions.create) - temperature: supported (Optional[float], between 0 and 2). [1] - max_tokens: supported but deprecated (and not compatible with o-series models). [2] - max_completion_tokens: supported (upper bound on tokens generated for a completion, including visible output and reasoning tokens). [2] gpt-5-mini (chat.completions.create) - temperature: not supported (gpt-5 series requires default temperature; user reports indicate temperature not supported/only certain defaults allowed). [3] - max_tokens: not supported (use max_completion_tokens instead). [4] - max_completion_tokens: supported (it is the replacement for max_tokens; request type supports it). [2][4] Notes on what “supported” means here - In the openai Python SDK, chat.completions.create exposes both temperature, max_tokens, and max_completion_tokens as request parameters at the type level; however, model-specific support still applies at runtime (e.g., max_tokens not compatible/not supported for some model families, and temperature not supported for GPT-5 mini per reported behavior). [2]
Citations:
- 1: https://developers.openai.com/api/reference/python/resources/chat/subresources/completions/methods/create/
- 2: https://github.com/openai/openai-python/blob/v1.106.1/src/openai/resources/chat/completions/completions.py
- 3: https://github.com/openai/openai-python/blob/722d3fff/src/openai/resources/chat/completions/completions.py
- 4: https://pypi.org/project/openai/
Forward generation parameters to OpenAI chat.completions.create
In ai_research/dataset_generation/infrastructure/openai_client.py (lines 73-76), the request only passes model and messages, so temperature and any token limits aren’t forwarded to OpenAI. That makes generation-related CLI options ineffective and can change output/cost behavior.
Map and pass the correct parameters per model (openai==1.109.1): gpt-4o-mini supports temperature and max_completion_tokens (while max_tokens is deprecated/incompatible for o-series), and gpt-5-mini doesn’t support temperature/max_tokens but does support max_completion_tokens.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ai_research/dataset_generation/infrastructure/openai_client.py` around lines
73 - 76, The chat completion call in openai_client.py currently only passes
model and messages; update the call in the method that uses
self._client.chat.completions.create to forward generation args from the client
(e.g., self.temperature, self.max_completion_tokens, self.max_tokens) and
conditionalize them by model: for "gpt-4o-mini" include temperature (if set) and
max_completion_tokens; for "gpt-5-mini" do not pass temperature or legacy
max_tokens but do pass max_completion_tokens when present; ensure you only pass
parameters supported by openai==1.109.1 to avoid incompatible fields.
Comment on lines
+106
to
+450
| class OpenAICodexTextClientTests(unittest.TestCase): | ||
| def setUp(self) -> None: | ||
| self._old_codex_home = os.environ.get("CODEX_HOME") | ||
|
|
||
| def tearDown(self) -> None: | ||
| if self._old_codex_home is None: | ||
| os.environ.pop("CODEX_HOME", None) | ||
| else: | ||
| os.environ["CODEX_HOME"] = self._old_codex_home | ||
|
|
||
| def test_template_rendering_matches_openai_client(self) -> None: | ||
| template = "Hello {{.Name}}, count {{.Count}}" | ||
| data = {"Name": "world", "Count": 3} | ||
|
|
||
| self.assertEqual( | ||
| render_prompt_template(template, data), | ||
| render_openai_prompt_template(template, data), | ||
| ) | ||
|
|
||
| def test_constructor_stores_compatibility_fields(self) -> None: | ||
| client = OpenAICodexTextClient( | ||
| api_key="", | ||
| base_url="https://example.test/root", | ||
| model="gpt-5-codex", | ||
| temperature=0.5, | ||
| max_tokens=456, | ||
| max_retries=2, | ||
| retry_delay_seconds=0.25, | ||
| ) | ||
|
|
||
| self.assertEqual(client.api_key, "") | ||
| self.assertEqual(client.base_url, "https://example.test/root") | ||
| self.assertEqual(client.model, "gpt-5-codex") | ||
| self.assertEqual(client.temperature, 0.5) | ||
| self.assertEqual(client.max_tokens, 456) | ||
| self.assertEqual(client.max_retries, 2) | ||
| self.assertEqual(client.retry_delay_seconds, 0.25) | ||
|
|
||
| def test_constructor_rejects_non_empty_api_key(self) -> None: | ||
| with self.assertRaisesRegex(ValueError, "use OpenAITextClient"): | ||
| OpenAICodexTextClient( | ||
| api_key="key", model="gpt-5-codex", temperature=1.0, max_tokens=1 | ||
| ) | ||
|
|
||
| def test_cli_build_client_uses_openai_for_non_empty_api_key(self) -> None: | ||
| args = argparse.Namespace( | ||
| api_key="key", | ||
| base_url=None, | ||
| model="gpt-4o-mini", | ||
| temperature=1.0, | ||
| max_tokens=1, | ||
| ) | ||
|
|
||
| self.assertIsInstance(_build_client(args), OpenAITextClient) | ||
|
|
||
| def test_cli_build_client_uses_codex_for_empty_api_key(self) -> None: | ||
| args = argparse.Namespace( | ||
| api_key="", | ||
| base_url=None, | ||
| model="gpt-5-codex", | ||
| temperature=1.0, | ||
| max_tokens=1, | ||
| ) | ||
|
|
||
| self.assertIsInstance(_build_client(args), OpenAICodexTextClient) | ||
|
|
||
| def test_infer_loads_chatgpt_file_auth_and_uses_codex_endpoint(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth(Path(tmp_dir)) | ||
| client = _FakeCodexClient( | ||
| streams=[ | ||
| _sse( | ||
| {"type": "response.output_text.delta", "delta": "ok"}, | ||
| {"type": "response.completed"}, | ||
| ) | ||
| ] | ||
| ) | ||
|
|
||
| self.assertEqual(client.infer("Hi {{.Name}}", {"Name": "there"}), "ok") | ||
|
|
||
| url, headers, body = client.requests[0] | ||
| self.assertEqual(url, f"{CODEX_BASE_URL}/responses") | ||
| self.assertEqual(headers["Authorization"], "Bearer access-token") | ||
| self.assertEqual(headers["Content-Type"], "application/json") | ||
| self.assertEqual(headers["Accept"], "text/event-stream") | ||
| self.assertEqual(headers["session_id"], client.conversation_id) | ||
| self.assertEqual(headers["ChatGPT-Account-ID"], "account-id") | ||
| input_items = cast(list[dict[str, Any]], body["input"]) | ||
| content_items = cast(list[dict[str, Any]], input_items[0]["content"]) | ||
| self.assertEqual(content_items[0]["text"], "Hi there") | ||
|
|
||
| def test_infer_accepts_missing_auth_mode_when_tokens_are_present(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth(Path(tmp_dir), auth_mode=None) | ||
| client = _FakeCodexClient( | ||
| streams=[ | ||
| _sse( | ||
| {"type": "response.output_text.delta", "delta": "ok"}, | ||
| {"type": "response.completed"}, | ||
| ) | ||
| ] | ||
| ) | ||
|
|
||
| self.assertEqual(client.infer("Hi", {}), "ok") | ||
|
|
||
| def test_base_url_overrides_codex_endpoint_root(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth(Path(tmp_dir), account_id=None) | ||
| client = _FakeCodexClient( | ||
| base_url="https://example.test/codex/", | ||
| streams=[ | ||
| _sse( | ||
| {"type": "response.output_text.delta", "delta": "ok"}, | ||
| {"type": "response.done"}, | ||
| ) | ||
| ], | ||
| ) | ||
|
|
||
| client.infer("Hi", {}) | ||
|
|
||
| self.assertEqual( | ||
| client.requests[0][0], "https://example.test/codex/responses" | ||
| ) | ||
| self.assertNotIn("ChatGPT-Account-ID", client.requests[0][1]) | ||
|
|
||
| def test_request_body_contains_no_tool_responses_shape(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth(Path(tmp_dir)) | ||
| client = _FakeCodexClient( | ||
| streams=[ | ||
| _sse( | ||
| {"type": "response.output_text.delta", "delta": "ok"}, | ||
| {"type": "response.completed"}, | ||
| ) | ||
| ] | ||
| ) | ||
|
|
||
| client.infer("Prompt", {}) | ||
|
|
||
| body = client.requests[0][2] | ||
| self.assertEqual(body["model"], "gpt-5-codex") | ||
| self.assertEqual(body["instructions"], "") | ||
| self.assertEqual(body["tools"], []) | ||
| self.assertEqual(body["tool_choice"], "auto") | ||
| self.assertFalse(body["parallel_tool_calls"]) | ||
| self.assertIsNone(body["reasoning"]) | ||
| self.assertFalse(body["store"]) | ||
| self.assertTrue(body["stream"]) | ||
| self.assertEqual(body["include"], []) | ||
| self.assertEqual(body["prompt_cache_key"], client.conversation_id) | ||
| self.assertNotIn("temperature", body) | ||
| self.assertNotIn("max_tokens", body) | ||
|
|
||
| def test_sse_parser_joins_output_deltas_and_accepts_done(self) -> None: | ||
| client = OpenAICodexTextClient( | ||
| api_key="", model="gpt-5-codex", temperature=1.0, max_tokens=1 | ||
| ) | ||
|
|
||
| result = client._parse_sse( | ||
| _sse( | ||
| {"type": "response.output_text.delta", "delta": "hello"}, | ||
| {"type": "response.output_text.delta", "delta": " world"}, | ||
| {"type": "response.done"}, | ||
| ) | ||
| ) | ||
|
|
||
| self.assertEqual(result, "hello world") | ||
|
|
||
| def test_sse_parser_falls_back_to_output_item_done_text(self) -> None: | ||
| client = OpenAICodexTextClient( | ||
| api_key="", model="gpt-5-codex", temperature=1.0, max_tokens=1 | ||
| ) | ||
|
|
||
| result = client._parse_sse( | ||
| _sse( | ||
| { | ||
| "type": "response.output_item.done", | ||
| "item": { | ||
| "type": "message", | ||
| "content": [{"type": "output_text", "text": "full text"}], | ||
| }, | ||
| }, | ||
| {"type": "response.completed"}, | ||
| ) | ||
| ) | ||
|
|
||
| self.assertEqual(result, "full text") | ||
|
|
||
| def test_transient_response_retries_with_exponential_backoff(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth(Path(tmp_dir)) | ||
| client = _FakeCodexClient( | ||
| streams=[ | ||
| _HttpStatusError(500, "server error"), | ||
| _HttpStatusError(429, "rate limit"), | ||
| _sse( | ||
| {"type": "response.output_text.delta", "delta": "ok"}, | ||
| {"type": "response.completed"}, | ||
| ), | ||
| ] | ||
| ) | ||
|
|
||
| self.assertEqual(client.infer("Prompt", {}), "ok") | ||
| self.assertEqual(len(client.requests), 3) | ||
| self.assertEqual(client.sleeps, [1.0, 2.0]) | ||
|
|
||
| def test_incomplete_sse_stream_retries_with_exponential_backoff(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth(Path(tmp_dir)) | ||
| client = _FakeCodexClient( | ||
| streams=[ | ||
| _sse({"type": "response.output_text.delta", "delta": "partial"}), | ||
| _sse( | ||
| {"type": "response.output_text.delta", "delta": "ok"}, | ||
| {"type": "response.completed"}, | ||
| ), | ||
| ] | ||
| ) | ||
|
|
||
| self.assertEqual(client.infer("Prompt", {}), "ok") | ||
| self.assertEqual(len(client.requests), 2) | ||
| self.assertEqual(client.sleeps, [1.0]) | ||
|
|
||
| def test_incomplete_sse_stream_aborts_after_exhausting_retries(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth(Path(tmp_dir)) | ||
| client = _FakeCodexClient( | ||
| streams=[ | ||
| _sse({"type": "response.output_text.delta", "delta": "one"}), | ||
| _sse({"type": "response.output_text.delta", "delta": "two"}), | ||
| _sse({"type": "response.output_text.delta", "delta": "three"}), | ||
| ], | ||
| max_retries=2, | ||
| ) | ||
|
|
||
| with self.assertRaisesRegex(RuntimeError, "Codex inference exhausted retries"): | ||
| client.infer("Prompt", {}) | ||
| self.assertEqual(len(client.requests), 3) | ||
| self.assertEqual(client.sleeps, [1.0, 2.0]) | ||
|
|
||
| def test_non_transient_400_does_not_retry(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth(Path(tmp_dir)) | ||
| client = _FakeCodexClient(streams=[_HttpStatusError(400, "bad request")]) | ||
|
|
||
| with self.assertRaisesRegex(RuntimeError, "non-retryable HTTP status 400"): | ||
| client.infer("Prompt", {}) | ||
| self.assertEqual(len(client.requests), 1) | ||
| self.assertEqual(client.sleeps, []) | ||
|
|
||
| def test_stale_chatgpt_auth_refreshes_before_inference_and_preserves_account_id( | ||
| self, | ||
| ) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| old_refresh = (datetime.now(timezone.utc) - timedelta(days=9)).isoformat() | ||
| _write_auth(Path(tmp_dir), last_refresh=old_refresh) | ||
| client = _FakeCodexClient( | ||
| streams=[ | ||
| _sse( | ||
| {"type": "response.output_text.delta", "delta": "ok"}, | ||
| {"type": "response.completed"}, | ||
| ) | ||
| ], | ||
| refresh_response={ | ||
| "access_token": "new-access", | ||
| "refresh_token": "new-refresh", | ||
| }, | ||
| ) | ||
|
|
||
| client.infer("Prompt", {}) | ||
|
|
||
| self.assertEqual(len(client.refresh_requests), 1) | ||
| self.assertEqual( | ||
| client.requests[0][1]["Authorization"], "Bearer new-access" | ||
| ) | ||
| auth_payload = json.loads((Path(tmp_dir) / "auth.json").read_text()) | ||
| self.assertEqual(auth_payload["tokens"]["account_id"], "account-id") | ||
| self.assertEqual(auth_payload["tokens"]["refresh_token"], "new-refresh") | ||
|
|
||
| def test_first_401_refreshes_once_and_retries_once(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth(Path(tmp_dir)) | ||
| client = _FakeCodexClient( | ||
| streams=[ | ||
| _HttpStatusError(401, "unauthorized"), | ||
| _sse( | ||
| {"type": "response.output_text.delta", "delta": "ok"}, | ||
| {"type": "response.completed"}, | ||
| ), | ||
| ], | ||
| refresh_response={ | ||
| "access_token": "new-access", | ||
| "refresh_token": "new-refresh", | ||
| }, | ||
| ) | ||
|
|
||
| self.assertEqual(client.infer("Prompt", {}), "ok") | ||
| self.assertEqual(len(client.refresh_requests), 1) | ||
| self.assertEqual(len(client.requests), 2) | ||
| self.assertEqual( | ||
| client.requests[1][1]["Authorization"], "Bearer new-access" | ||
| ) | ||
|
|
||
| def test_errors_are_sanitized(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp_dir: | ||
| os.environ["CODEX_HOME"] = tmp_dir | ||
| _write_auth( | ||
| Path(tmp_dir), | ||
| access_token="secret-access", | ||
| refresh_token="secret-refresh", | ||
| ) | ||
| client = _FakeCodexClient( | ||
| streams=[_HttpStatusError(400, "secret-access secret-refresh")] | ||
| ) | ||
|
|
||
| with self.assertRaises(RuntimeError) as exc: | ||
| client.infer("Prompt", {}) | ||
|
|
||
| message = str(exc.exception) | ||
| self.assertNotIn("secret-access", message) | ||
| self.assertNotIn("secret-refresh", message) | ||
|
|
||
| def test_sse_errors_are_sanitized(self) -> None: | ||
| client = OpenAICodexTextClient( | ||
| api_key="", model="gpt-5-codex", temperature=1.0, max_tokens=1 | ||
| ) | ||
|
|
||
| with self.assertRaisesRegex(RuntimeError, "invalid JSON") as exc: | ||
| client._parse_sse([b"data: {token-secret\n", b"\n"]) | ||
|
|
||
| self.assertNotIn("token-secret", str(exc.exception)) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
There was a problem hiding this comment.
tests/ suite must be integration-style against real IntentGuard API.
This test class is unit-style (mock/fake client, private-method verification) and does not satisfy the repository’s test policy for files under tests/.
As per coding guidelines, tests/**/*.py: Use integration-style tests in tests/ directory with the real IntentGuard API.
🧰 Tools
🪛 Ruff (0.15.13)
[error] 424-424: Possible hardcoded password assigned to argument: "access_token"
(S106)
[error] 425-425: Possible hardcoded password assigned to argument: "refresh_token"
(S106)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_openai_codex_client.py` around lines 106 - 450, This test file
contains unit-style tests (class OpenAICodexTextClientTests) that use
_FakeCodexClient and call private helpers like OpenAICodexTextClient._parse_sse;
change them to integration-style tests that exercise the real IntentGuard API
and public client surface: remove or rewrite tests that use _FakeCodexClient,
_sse, or call private methods (e.g., _parse_sse) and instead instantiate the
real client via the public factory (_build_client or
OpenAITextClient/OpenAICodexTextClient with real credentials), perform live
infer calls against the IntentGuard endpoint, assert end-to-end behavior (HTTP
headers, streaming output, retries, token refresh) using real responses, and
update setup/teardown to rely on real CODEX_HOME/credentials rather than writing
fake auth files. Ensure no tests import or assert internal private helpers or
fakes and that all assertions target public behavior only.
kdunee
force-pushed
the
dataset-generation-improvements
branch
from
May 24, 2026 18:36
3554f57 to
a133826
Compare
Route dataset generation through Codex auth when no API key is provided, add prompt-artifact filtering during transform, and simplify OpenAI chat requests. Cover Codex streaming, token refresh, retry behavior, client selection, logging, and artifact rejection with tests.
kdunee
force-pushed
the
dataset-generation-improvements
branch
from
May 24, 2026 18:38
a133826 to
eb99d1f
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Route dataset generation through Codex auth when no API key is provided, add prompt-artifact filtering during transform, and simplify OpenAI chat requests. Cover Codex streaming, token refresh, retry behavior, client selection, logging, and artifact rejection with tests.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes