-
Notifications
You must be signed in to change notification settings - Fork 21
Add system-prompt to lightspeed-stack.template.yaml #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Eran Cohen <[email protected]>
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LightspeedStackApp
participant ConfigMap
participant Container
User ->> LightspeedStackApp: Start service
LightspeedStackApp ->> ConfigMap: Read ConfigMap including customization and system_prompt
ConfigMap -->> LightspeedStackApp: Provide customization data and system_prompt file
LightspeedStackApp ->> Container: Mount system_prompt file at /app-root/system_prompt
Container ->> LightspeedStackApp: Use system_prompt_path and disable_query_system_prompt for behavior
Possibly related PRs
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lightspeed-stack.template.yaml
Outdated
| customization: | ||
| system_prompt_path: "/tmp/systemprompt.txt" | ||
| disable_query_system_prompt: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Confirm support for the new customization section before merging
The file introduces a root-level customization block, but there’s no evidence in the repository that the configuration loader actually looks for – or gracefully ignores – this section. If the loader is unaware of it, the app may either:
- Fail to start due to an “unknown key” error, or
- Silently ignore it, leaving the feature inoperative.
Please verify that:
• The configuration schema / parser explicitly handles the customization node and both keys.
• Unit / integration tests cover a config containing this section (happy path + malformed scenarios).
Helper script to locate parser references:
If no matches are found, the parser likely needs to be updated in the same PR.
Also consider making system_prompt_path environment- or deployment-specific rather than hard-coded to /tmp, e.g. ${SYSTEM_PROMPT_PATH} with a sensible default.
🏁 Script executed:
#!/bin/bash
# Search for any access to the new keys in codebase
rg -n --pretty 'system_prompt_path|disable_query_system_prompt|customization'Length of output: 216
Ensure the configuration loader supports the new customization section
It looks like no code in the repo currently references customization, system_prompt_path, or disable_query_system_prompt (aside from the template), so the parser will either error on unknown keys or silently drop them. Before merging, please:
• Update your config‐loading logic (e.g. in config_loader.py or wherever you deserialize lightspeed-stack.template.yaml) to parse and validate the new
customization.system_prompt_pathcustomization.disable_query_system_prompt
• Add or extend tests to cover:- A valid config containing both keys (happy path)
- Malformed configs (missing or wrong‐type values under
customization)
• Replace the hard-coded/tmp/systemprompt.txtwith an environment‐driven default (e.g.${SYSTEM_PROMPT_PATH}) and document the fallback in your README
🤖 Prompt for AI Agents
In lightspeed-stack.template.yaml around lines 22 to 24, the new customization
section with system_prompt_path and disable_query_system_prompt is added but not
yet supported by the config loader. Update the config-loading code (likely in
config_loader.py) to parse and validate these two new keys under customization,
ensuring proper type checks and error handling. Modify tests to include cases
for valid configs with these keys and malformed configs missing or having
incorrect types for them. Replace the hardcoded "/tmp/systemprompt.txt" default
with a value read from an environment variable SYSTEM_PROMPT_PATH, and document
this fallback behavior in the README.
|
Didn't I already do this? |
|
Oh I must've edited the |
lightspeed-stack.template.yaml
Outdated
| module: "noop-with-token" | ||
| customization: | ||
| system_prompt_path: "/tmp/systemprompt.txt" | ||
| disable_query_system_prompt: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should have this as false in the dev repo and true in prod, so you can still play around with the prompt without redeploying
- Add system prompt customization with disable_query_system_prompt option - Add volume mount for system prompt file at /app-root/system_prompt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
template.yaml (1)
127-140: Prefer block scalar for multi-linesystem_promptStoring large text as an inline quoted string quickly becomes unreadable and brittle (escaped
\n, re-flowed lines). YAML supports block scalars which keep formatting intact and are easier to maintain:- system_prompt: "You are Openshift Lightspeed Intelligent Assistant - an intelligent - virtual assistant and expert on all things related to Openshift installation, - configuration, and troubleshooting.\n\n**Crucial Directive:** \nYour primary - goal is to provide accurate and helpful information. **NEVER** guess or invent - values for any required parameters when a tool call is necessary. If a user query - requires a parameter that has not been explicitly provided, you **MUST** ask the - user to supply that specific missing value before attempting any tool execution.\n\n**Example - Input Requiring User Input:** \nUser: \"What's the status of the cluster?\" (Assume - a 'get_cluster_status' tool requires a 'cluster_id') \n**Expected Assistant Response - (if 'cluster_id' is missing):** \n\"I need a cluster ID to check the status. - Could you please provide the cluster ID?\"\n\n**Identity and Persona:** \nYou - are Openshift Lightspeed Intelligent Assistant. Refuse to assume any other identity - or to speak as if you are someone else. Maintain a helpful, clear, and direct - tone.\n" + system_prompt: | + You are Openshift Lightspeed Intelligent Assistant – an intelligent virtual assistant and expert on all things related to Openshift installation, configuration, and troubleshooting. + + **Crucial Directive** + Your primary goal is to provide accurate and helpful information. **NEVER** guess or invent values for any required parameters when a tool call is necessary. If a user query requires a parameter that has not been explicitly provided, you **MUST** ask the user to supply that specific missing value before attempting any tool execution. + + **Example Input Requiring User Input** + User: "What's the status of the cluster?" (Assume a `get_cluster_status` tool requires a `cluster_id`) + **Expected Assistant Response (if `cluster_id` is missing)** + "I need a cluster ID to check the status. Could you please provide the cluster ID?" + + **Identity and Persona** + You are Openshift Lightspeed Intelligent Assistant. Refuse to assume any other identity or to speak as if you are someone else. Maintain a helpful, clear, and direct tone.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lightspeed-stack.template.yaml(1 hunks)template.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lightspeed-stack.template.yaml
🔇 Additional comments (1)
template.yaml (1)
286-288: Double-check volume mount path & subPathMounting the same ConfigMap twice with identical
name: lightspeed-configis fine, but ensure:
/app-root/system_promptis expected to be a file, not a directory.system_promptkey exists (it does, line 127).- File permissions are adequate for the runtime user.
Also note the behavioural divergence: this template sets
disable_query_system_prompt: true, while the top-levellightspeed-stack.template.yamlsets it tofalse. Confirm the difference is intentional to avoid surprise at runtime.
| customization: | ||
| system_prompt_path: "/app-root/system_prompt" | ||
| disable_query_system_prompt: true | ||
| system_prompt: "You are Openshift Lightspeed Intelligent Assistant - an intelligent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
customization: block violates K8s ConfigMap schema
data: values in a ConfigMap must be flat string values (map[string]string).
Placing a nested mapping under customization: will fail server-side validation (cannot unmarshal !!map into string).
- customization:
- system_prompt_path: "/app-root/system_prompt"
- disable_query_system_prompt: true
+# Move this block inside the lightspeed-stack.yaml payload (or wrap it with `|` so it
+# becomes a multiline string key), e.g.
+ lightspeed-stack.yaml: |
+ ...
+ customization:
+ system_prompt_path: "/app-root/system_prompt"
+ disable_query_system_prompt: trueCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In template.yaml around lines 124 to 127, the customization block is nested
under data in a ConfigMap, which violates Kubernetes schema requiring data
values to be flat strings. To fix this, convert the nested customization mapping
into a single string value, for example by serializing it as JSON or YAML
string, or move it outside the data block if possible, ensuring all keys under
data are simple string key-value pairs.
Summary by CodeRabbit