(feat): Auto rag migrate to responses api phase1#115
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR removes the deployment prepare_responses component and its docs/tests/OWNERS entries; refactors training/rag_templates_optimization to load templates from components/training/autorag/shared and replace OGX SSL handling with an httpx probe/retry that can instantiate OgxClient with verify=False for self-signed certs; changes pattern JSON to emit settings.vector_store_binding and settings.responses_template; adds shared ogx_indexing and ogx_inference notebook templates and a create_model_response CLI script template; updates unit tests to mock httpx probes; removes the prepare-responses pipeline step; and updates pyproject packaging and package-data. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@components/training/autorag/rag_templates_optimization/component.py`:
- Around line 696-700: In _build_pattern_json replace the incorrect use of the
built-in eval with the function parameter name: use evaluation_result.collection
(and any other properties accessed via eval) to reference the passed-in
evaluation_result; update the "vector_store_binding" dict entry (and any other
occurrences inside _build_pattern_json) so provider_id uses
vector_io_provider_id, provider_type uses getattr(provider, "provider_type",
"Unknown"), and vector_store_id uses evaluation_result.collection to avoid
shadowing the built-in eval.
- Around line 671-679: The variable generation_system_message_text is
accidentally created as a 1-tuple because of the trailing comma in the
generation.get call; change the assignment so generation_system_message_text is
a plain string (remove the extra comma/tuple wrapping around the default value)
so that responses_template.instructions and generation.system_message_text
receive a string rather than a tuple.
In
`@components/training/autorag/shared/notebook_templates/ogx_indexing_template.ipynb`:
- Around line 316-342: The call to create_ogx_client is passing the arguments in
reverse and re-reading env vars (discarding the getpass/default fallback);
change the invocation so it uses the already-resolved variables for base URL and
API key (the variables created earlier) and pass them in the correct order
matching the signature create_ogx_client(base_url, api_key) so the SSL probe
gets the URL and the OgxClient receives the API key.
- Around line 326-340: The except block for httpx.ConnectError in the OGX client
initialization currently only handles self-signed-certificate errors and
silently falls through for other ConnectError cases; update the except
httpx.ConnectError as e branch (around the httpx.get(base_url) probe and the
return OgxClient(...) logic) to re-raise the caught exception when the regex
search(r"\\bself.*signed.*certificate\\b", str(e)) does not match, so that
non-cert connection failures (DNS/refused/etc.) are not masked—keep the existing
self-signed handling and only return an OgxClient with verify=False when the
regex matches and the verify-free probe succeeds.
In
`@components/training/autorag/shared/notebook_templates/ogx_inference_template.ipynb`:
- Around line 105-131: The create_ogx_client call has its arguments swapped and
swallows non-self-signed ConnectError; fix by calling create_ogx_client with
(base_url, api_key) using os.getenv("OGX_CLIENT_BASE_URL") then
os.getenv("OGX_CLIENT_API_KEY") (so the getpass/default logic for
OGX_CLIENT_BASE_URL is respected), and inside create_ogx_client ensure the
except httpx.ConnectError as e branch only handles self-signed certificates
(matching via search) and re-raise the original ConnectError for all other cases
instead of suppressing it.
In
`@components/training/autorag/shared/script_templates/create_model_response.py.templ`:
- Around line 67-70: The code directly indexes assistant_last_message[0] and
output_text[0] after building them from response.json(), which can raise
IndexError/KeyError on unexpected payloads; update the parsing around
response.json(), assistant_last_message and output_text to validate the
structure before indexing (check that "output" exists and is a list, that there
is at least one message with role == "assistant", and that that message has a
"content" list containing an object with "type" == "output_text"), and handle
missing/empty cases by logging a clear error or falling back to a safe default
instead of indexing blindly.
- Around line 86-91: The except Exception block currently calls client.close()
but then allows the outer while loop to continue, causing subsequent
client.post(...) calls to run against a closed client; either remove the
client.close() from that except so the final client.close() after the loop
handles cleanup, or if the intent is to stop processing on any unexpected error,
call client.close() and then break or return immediately from the loop; locate
the while loop and the except Exception that references client.close() (and the
client.post calls) and implement one of these fixes so iterations never continue
using a closed client.
In `@pyproject.toml`:
- Line 94: The pyproject.toml lists
"kfp_components.components.training.autorag.shared" but that package isn't
discovered, causing CI failures; either add a Python package marker by creating
components/training/autorag/shared/__init__.py so discovery finds the package
(if it should be importable), or remove the
"kfp_components.components.training.autorag.shared" entry from pyproject.toml
and instead include its non-code assets via an existing discovered package
(e.g., kfp_components.components.training.autorag) using
package_data/MANIFEST.in or the pyproject include mechanism; update only the
package entry or add the __init__.py accordingly so package discovery passes.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b7fae005-309e-42be-9eee-27f5739b14a4
📒 Files selected for processing (23)
components/deployment/autorag/OWNERScomponents/deployment/autorag/README.mdcomponents/deployment/autorag/__init__.pycomponents/deployment/autorag/build_responses_request_bodies/OWNERScomponents/deployment/autorag/build_responses_request_bodies/README.mdcomponents/deployment/autorag/build_responses_request_bodies/__init__.pycomponents/deployment/autorag/build_responses_request_bodies/component.pycomponents/deployment/autorag/build_responses_request_bodies/example_pipelines.pycomponents/deployment/autorag/build_responses_request_bodies/metadata.yamlcomponents/deployment/autorag/build_responses_request_bodies/tests/__init__.pycomponents/deployment/autorag/build_responses_request_bodies/tests/openai_responses_request_validate.pycomponents/deployment/autorag/build_responses_request_bodies/tests/test_component_unit.pycomponents/deployment/autorag/build_responses_request_bodies/tests/test_openai_responses_api_compliance.pycomponents/training/autorag/rag_templates_optimization/component.pycomponents/training/autorag/rag_templates_optimization/notebook_templates/ogx_indexing_template.ipynbcomponents/training/autorag/rag_templates_optimization/notebook_templates/ogx_inference_template.ipynbcomponents/training/autorag/rag_templates_optimization/tests/test_component_unit.pycomponents/training/autorag/shared/notebook_templates/ogx_indexing_template.ipynbcomponents/training/autorag/shared/notebook_templates/ogx_inference_template.ipynbcomponents/training/autorag/shared/script_templates/create_model_response.py.templpipelines/training/autorag/documents_rag_optimization_pipeline/pipeline.pypipelines/training/autorag/documents_rag_optimization_pipeline/tests/test_pipeline_unit.pypyproject.toml
💤 Files with no reviewable changes (17)
- components/deployment/autorag/build_responses_request_bodies/OWNERS
- components/deployment/autorag/init.py
- components/deployment/autorag/OWNERS
- components/deployment/autorag/build_responses_request_bodies/README.md
- components/deployment/autorag/build_responses_request_bodies/init.py
- components/deployment/autorag/README.md
- components/training/autorag/rag_templates_optimization/notebook_templates/ogx_indexing_template.ipynb
- components/training/autorag/rag_templates_optimization/notebook_templates/ogx_inference_template.ipynb
- pipelines/training/autorag/documents_rag_optimization_pipeline/tests/test_pipeline_unit.py
- components/deployment/autorag/build_responses_request_bodies/metadata.yaml
- components/deployment/autorag/build_responses_request_bodies/component.py
- components/deployment/autorag/build_responses_request_bodies/tests/openai_responses_request_validate.py
- components/deployment/autorag/build_responses_request_bodies/example_pipelines.py
- pipelines/training/autorag/documents_rag_optimization_pipeline/pipeline.py
- components/deployment/autorag/build_responses_request_bodies/tests/test_openai_responses_api_compliance.py
- components/deployment/autorag/build_responses_request_bodies/tests/init.py
- components/deployment/autorag/build_responses_request_bodies/tests/test_component_unit.py
b7218c2 to
f7b7fd3
Compare
|
/retest |
|
@MichalSteczko please run e2e tests including generated py script and responses template under pattern.json. |
There was a problem hiding this comment.
Fix the key error
Please add general error handling inside while true loop to avoid infinite looping.
Please add the messages of the caught errors.
Add the SSL error handling while verification is not disabled and server is not supporting verified connection.
Artifacts generated by the pipeline (FYI @LukaszCmielowski):
responses_artifacts.zip
There was a problem hiding this comment.
Looks like pattern.json is still incorrect:
- nulls are still there
"timeout": null,
"model_type": null,
"provider_id": null,
"provider_resource_id": null
}
},
- Retrieval method is simple with hybrid params; however in the responses template there are no hybrid params attached:
"retrieval": {
"method": "simple",
"number_of_chunks": 3,
"search_mode": "hybrid",
"ranker_strategy": "rrf",
"ranker_k": 60,
"ranker_alpha": 1
},
"tools": [
{
"type": "file_search",
"vector_store_ids": [
"vs_c40a084f-6eca-43bf-8184-a0c5ad0d5308"
]
}
],
-
responses template the instruction seems like a result of hallucination - please check what was there in 3.4
-
a) stream and store I'd suspect it should be vice versa b) make sure input as flat text works; wouldn't it be better to have messages passed there as it was in 3.4 ?. That will allow to have system message and instruction if required.
"stream": false,
"store": true,
"input": "<user_query_placeholder>",
d2062b0 to
5d48957
Compare
Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com>
Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com>
…ponents Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com>
Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com>
…ierarchy restructuring; project metadata related update Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com>
… ADR doc Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com>
Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com>
Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com>
5d48957 to
2a2044f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/training/autorag/shared/notebook_templates/ogx_indexing_template.ipynb (1)
342-342:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
getpassfallback is still discarded—os.getenv()bypasses resolved variables.Line 314 resolves
OGX_CLIENT_API_KEYwith agetpassfallback, but line 342 callsos.getenv("OGX_CLIENT_API_KEY")directly. If the env var is unset,os.getenv()returnsNoneand the interactive prompt is never used. Same issue forOGX_CLIENT_BASE_URLdefault.The previous review fixed the argument order but not the variable usage.
🐛 Use the resolved variables
-client = create_ogx_client(os.getenv("OGX_CLIENT_BASE_URL"), os.getenv("OGX_CLIENT_API_KEY")) +client = create_ogx_client(OGX_CLIENT_BASE_URL, OGX_CLIENT_API_KEY)🤖 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 `@components/training/autorag/shared/notebook_templates/ogx_indexing_template.ipynb` at line 342, The call to create_ogx_client currently uses os.getenv("OGX_CLIENT_BASE_URL") and os.getenv("OGX_CLIENT_API_KEY") which bypasses the previously resolved values (including the getpass fallback); update the invocation of create_ogx_client to pass the resolved variables (e.g., the local variables that were set earlier for the base URL and API key, such as ogx_base_url and ogx_api_key or whatever names were used at line 314) instead of calling os.getenv() again so the interactive fallback is honored.
🤖 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
`@components/training/autorag/shared/notebook_templates/ogx_indexing_template.ipynb`:
- Around line 326-331: The httpx.get calls in the notebook (the plain
httpx.get(base_url) probe and the retry httpx.get(base_url, verify=False)) lack
timeouts and can hang; update both calls to pass a sensible timeout (e.g.,
timeout=5 or timeout=10 seconds) so the requests fail fast on unresponsive
endpoints and handle the resulting exceptions consistently (preserving the
existing ConnectError handling flow).
---
Duplicate comments:
In
`@components/training/autorag/shared/notebook_templates/ogx_indexing_template.ipynb`:
- Line 342: The call to create_ogx_client currently uses
os.getenv("OGX_CLIENT_BASE_URL") and os.getenv("OGX_CLIENT_API_KEY") which
bypasses the previously resolved values (including the getpass fallback); update
the invocation of create_ogx_client to pass the resolved variables (e.g., the
local variables that were set earlier for the base URL and API key, such as
ogx_base_url and ogx_api_key or whatever names were used at line 314) instead of
calling os.getenv() again so the interactive fallback is honored.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3e9bd6fc-9d30-4796-8c02-ee2035df494c
📒 Files selected for processing (24)
components/deployment/autorag/OWNERScomponents/deployment/autorag/README.mdcomponents/deployment/autorag/__init__.pycomponents/deployment/autorag/build_responses_request_bodies/OWNERScomponents/deployment/autorag/build_responses_request_bodies/README.mdcomponents/deployment/autorag/build_responses_request_bodies/__init__.pycomponents/deployment/autorag/build_responses_request_bodies/component.pycomponents/deployment/autorag/build_responses_request_bodies/example_pipelines.pycomponents/deployment/autorag/build_responses_request_bodies/metadata.yamlcomponents/deployment/autorag/build_responses_request_bodies/tests/__init__.pycomponents/deployment/autorag/build_responses_request_bodies/tests/openai_responses_request_validate.pycomponents/deployment/autorag/build_responses_request_bodies/tests/test_component_unit.pycomponents/deployment/autorag/build_responses_request_bodies/tests/test_openai_responses_api_compliance.pycomponents/training/autorag/rag_templates_optimization/component.pycomponents/training/autorag/rag_templates_optimization/notebook_templates/ogx_indexing_template.ipynbcomponents/training/autorag/rag_templates_optimization/notebook_templates/ogx_inference_template.ipynbcomponents/training/autorag/rag_templates_optimization/tests/test_component_unit.pycomponents/training/autorag/shared/__init__.pycomponents/training/autorag/shared/notebook_templates/ogx_indexing_template.ipynbcomponents/training/autorag/shared/notebook_templates/ogx_inference_template.ipynbcomponents/training/autorag/shared/script_templates/create_model_response.py.templpipelines/training/autorag/documents_rag_optimization_pipeline/pipeline.pypipelines/training/autorag/documents_rag_optimization_pipeline/tests/test_pipeline_unit.pypyproject.toml
💤 Files with no reviewable changes (17)
- components/deployment/autorag/README.md
- components/deployment/autorag/build_responses_request_bodies/README.md
- components/deployment/autorag/build_responses_request_bodies/metadata.yaml
- components/deployment/autorag/build_responses_request_bodies/example_pipelines.py
- pipelines/training/autorag/documents_rag_optimization_pipeline/tests/test_pipeline_unit.py
- components/deployment/autorag/build_responses_request_bodies/tests/test_component_unit.py
- components/deployment/autorag/build_responses_request_bodies/init.py
- pipelines/training/autorag/documents_rag_optimization_pipeline/pipeline.py
- components/deployment/autorag/OWNERS
- components/deployment/autorag/build_responses_request_bodies/OWNERS
- components/training/autorag/rag_templates_optimization/notebook_templates/ogx_indexing_template.ipynb
- components/deployment/autorag/build_responses_request_bodies/tests/test_openai_responses_api_compliance.py
- components/deployment/autorag/build_responses_request_bodies/component.py
- components/deployment/autorag/init.py
- components/training/autorag/rag_templates_optimization/notebook_templates/ogx_inference_template.ipynb
- components/deployment/autorag/build_responses_request_bodies/tests/init.py
- components/deployment/autorag/build_responses_request_bodies/tests/openai_responses_request_validate.py
🚧 Files skipped from review as they are similar to previous changes (5)
- components/training/autorag/shared/script_templates/create_model_response.py.templ
- pyproject.toml
- components/training/autorag/shared/notebook_templates/ogx_inference_template.ipynb
- components/training/autorag/rag_templates_optimization/tests/test_component_unit.py
- components/training/autorag/rag_templates_optimization/component.py
Recent successful run with the changes from the PR: https://rh-ai.apps.rosa.ai-eng-gpu.socc.p3.openshiftapps.com/develop-train/pipelines/runs/ai-eng-cracow/runs/f6258582-d8af-4041-9b7d-3807e2518a36 producing the attached artifacts (including only relevant): |
Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
There was a problem hiding this comment.
There are still some issues in pattern.json:
- "instructions" is missing the first sentence from the "system_message_text" about the language.
- There is mismatch in hybrid params "retrieval" has both properties ranker_k and ranker_alpha - I suspect only one is required here and responses proeprty is correct - to be confirmed where the fix should go
- I got the feeling that in 3.4 the instruction was containing the prompt telling that file_search result should be used to create a final answer - to be confirmed.
- Could you please share the responses_body.json content for reference ? (before this PR changes) - If I'm not mistaken there were messages template used instead of flat
instructions.
|
|
Looks fine to me, but I'd like to hold off on official approval until we hear back from @LukaszCmielowski. |
DorotaDR
left a comment
There was a problem hiding this comment.
Unfortunately I need to request changes once again as I noticed an error during tests on cluster with self-signed cert
…-managed clusters Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
…on self-managed clusters Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Filip Komarzyniec <fkomarzy@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DorotaDR, LukaszCmielowski, MichalSteczko The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@nsingla fyi - there are changes in pyproject.toml related to autox components. |
|
/ok-to-test |
|
/ok-to-test cancel |
|
/ok-to-test |
|
/lgtm |
|
/lgtm cancel |
|
/lgtm |
4cab58f
into
opendatahub-io:main
Description of your changes:
Aligned produced
pattern.jsonfile to the ADR document: https://github.com/LukaszCmielowski/architecture-decision-records/blob/autox_docs_updates/documentation/components/autorag/features/rag_pattern_inference.md, meaning:responses_templatekey representing the payload for Responses API calls,vector_storetovectore_store_bindingkey with related changescreate_model_responseartifact (an interactive script) to each generated patternbuild_responses_requestartifactSuccessful pipeline run: https://rh-ai.apps.rosa.ai-eng-gpu.socc.p3.openshiftapps.com/develop-train/pipelines/runs/ai-eng-cracow/runs/b213397d-0151-43d2-a30c-35bcba7e7482
Checklist:
Pre-Submission Checklist
Learn more about the pull request title convention used in this repository.
Additional Checklist Items for New or Updated Components/Pipelines
metadata.yamlincludes freshlastVerifiedtimestampare present and complete
snake_casenaming conventionSummary by CodeRabbit
Removed Features
New Features
Improvements