feat(intelligent-assistant): add optional OKP document retrieval - #310
maysunfaisal wants to merge 5 commits into
Conversation
a5384ad to
71536eb
Compare
|
/fs-review |
| ## Increasing Container Runtime Memory | ||
|
|
||
| If you encounter out-of-memory issues with the Lightspeed Core container, you can increase the memory available to your Podman or Docker virtual machine: | ||
| If you encounter out-of-memory issues with the Lightspeed Core or OKP container, you can increase the memory available to your Podman or Docker virtual machine: |
There was a problem hiding this comment.
Do we have a recommended amount of resources?
There was a problem hiding this comment.
We don’t currently have a validated minimum. I updated the documentation to explain that OKP and LCORE increase memory usage and that 8 GiB is only an example for troubleshooting, not a requirement. The Kubernetes OKP doesnt translate to Podman requirements, so I framed it as something users can do to alleviate memory issues
| # then re-uncomment provider blocks | ||
| ``` | ||
|
|
||
| Some upstream refs still ship the OKP block commented with a TODO. When syncing one of those refs, preserve or reactivate the `rag.okp` and `rag.retrieval.tool.sources: [okp]` block used by this Compose integration. |
There was a problem hiding this comment.
Is this true? AFAIK the only 3 locations where OKP was commented out were in this repo, Operator, and Helm, but if you ran the sync script to re-fetch the LCORE config file it would wipe out that comment
There was a problem hiding this comment.
You’re right. lightspeed-stack.yaml should remain a verbatim copy. I replaced the confusing wording drafted by Codex..
| disable_intelligent_assistant: | ||
| description: "Disable Intelligent Assistant services when registry.redhat.io credentials are unavailable" | ||
| required: false | ||
| default: "false" |
There was a problem hiding this comment.
Wonder if we should just have OKP disabled always in CI, it's a pretty large image?
There was a problem hiding this comment.
Agreed, the OKP image is not pulled in CI. Regular jobs disable IA and OKP, while the dedicated intelligent-assistant-without-okp job keeps LCORE enabled but profiles out OKP..
| ## Quick Start | ||
|
|
||
| 1. **Prerequisites**: [Podman](https://podman.io/docs/installation) v5.4.1+ (recommended) or [Docker](https://docs.docker.com/engine/) v28.1.0+ with Compose support | ||
| 1. **Prerequisites**: [Podman](https://podman.io/docs/installation) v5.4.1+ (recommended) or [Docker](https://docs.docker.com/engine/) v28.1.0+ with Compose support, plus a [Red Hat registry login](https://access.redhat.com/RegistryAuthentication) for the bundled OKP image unless you [run Intelligent Assistant without OKP](./docs/intelligent-assistant/working-with-intelligent-assistant.md#running-without-okp) |
There was a problem hiding this comment.
So this raises the barrier to entry for users as it would now require registry.redhat.io authentication by default.
Is that okay, @benwilcock @kadel ?
So far, we have been using only public images OOTB and, when auth is required like here with PostgreSQL, it was intentionally opt-in. But I understand, as already discussed on other PRs, that OKP does not have public images at this time: https://redhat.atlassian.net/browse/RHOKP-1792
BTW, in the Helm Chart, OKP seems effectively opt-in (at least on K8s, gated on a configured ingress host): https://github.com/redhat-developer/rhdh-chart/blob/94c20cf8031f0f46b82764b6eba90de180309fa0/charts/rhdh/templates/_helpers.tpl#L321-L331
Shouldn't it be opt-in in RHDH Local as well?
There was a problem hiding this comment.
@rm3l Correct—this PR currently enables OKP by default and provides compose.okp-disabled.override.example.yaml as an explicit opt-out that keeps IA enabled.
I agree this introduces a registry.redhat.io authentication requirement to the default experience. We can invert this so IA without OKP is the default, while users explicitly opt in to OKP through a Compose override after authenticating.
There was a problem hiding this comment.
Yeah, that sounds like a better approach IMO. Making IA-without-OKP the default keeps the clone-and-run experience intact as currently, and users who want OKP can opt in after authenticating against registry.redhat.io. We can revisit this to enable OKP by default once https://redhat.atlassian.net/browse/RHOKP-1792 is resolved.
There was a problem hiding this comment.
@rm3l I have updated as per the Slack conversation, OKP is disabled by default, PTAL.
a37117e to
f288dfc
Compare
|
/agentic_review |
Code Review by Qodo
1. Default document lookup endpoint fails
|
| composeConfig: | ||
| - name: "default" | ||
| cliArgs: "" | ||
| cliArgs: "-f compose.yaml" |
There was a problem hiding this comment.
| cliArgs: "-f compose.yaml" | |
| cliArgs: "" |
Shouldn't change anything (as long as the default compose file isn't renamed), but this was actually intentional to test the OOTB case of users simply running podman|docker compose up without any -f ...
There was a problem hiding this comment.
Updated, can you PTAL
| @@ -0,0 +1,39 @@ | |||
| # Copy this file to compose.override.yaml to enable OKP-backed product documentation. | |||
There was a problem hiding this comment.
Since OKP is now opt-in, I'd recommend using the existing compose overlay pattern that we leverage (see orchestrator, postgresql, corporate proxy compose files).
If possible, it would be much simpler IMO for users to just run something like podman compose -f compose.yaml -f intelligent-assistant/compose-with-okp.yaml up -d to get OKP.
It would also make the OKP overlay easier to test in CI (instead of all the conditional steps you currently have in action.yaml), I think. And also composable with other overlays if needed, like run RHDH Local with both OKP and Orchestrator enabled.
There was a problem hiding this comment.
Updated, can you PTAL
| ENCRYPT: "false" | ||
| ports: # dclint disable-line no-unbound-port-interfaces | ||
| - "8081:8080" | ||
| - "8983:8983" |
There was a problem hiding this comment.
| - "8983:8983" | |
| - "127.0.0.1:8983:8983" |
This Solr admin UI should ideally be bound to the loopback interface only. No reason to be network-exposed IMO.
There was a problem hiding this comment.
Updated, can you PTAL
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
f288dfc to
d730540
Compare
|



Target branch
Description
This PR integrates Offline Knowledge Portal (OKP) as an optional product-documentation source for Developer Hub Intelligent Assistant in RHDH Local.
registry.redhat.ioor pulling the OKP image.Which issue(s) does this PR fix or relate to
https://redhat.atlassian.net/browse/RHIDP-14136
PR acceptance criteria
How to test changes / Special notes to the reviewer
Intelligent Assistant without OKP (default)
Create a local provider configuration and
.envfile:Uncomment an inference provider in
configs/extra-files/lightspeed-stack.local.yaml, configure its credentials in.env, and add:Start RHDH Local without logging in to
registry.redhat.io:Verify
rhdhandlightspeed-coreare running, nookpcontainer exists, and the following commands return no output:Open http://localhost:7007/intelligent-assistant and verify the chatbot works without OKP-backed documentation or citations.
Intelligent Assistant with OKP (opt-in)
Stop the default deployment and authenticate to the Red Hat registry:
Enable OKP and create an OKP-enabled local provider configuration:
Uncomment the same inference provider in
configs/extra-files/lightspeed-stack-okp.local.yaml, then add to.env:Start and verify the opt-in deployment:
Verify
rhdh,lightspeed-core, andokpare running,okpis healthy, and LCORE has the OKP URL:podman exec lightspeed-core printenv OKP_SERVICE_URLOpen http://localhost:7007/intelligent-assistant, ask an RHDH documentation question, and verify the response contains clickable OKP citations.
Docker users can replace
podmanwithdockerin the commands above.Cleanup