revamped prompt action #2232
revamped prompt action #2232
Conversation
WalkthroughThis PR replaces per-prompt Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Config as Action Config
participant Model as PromptAction (model)
participant Proc as Prompt Processor
participant LLM as LLM Executor
rect rgb(245,250,255)
Config->>Model: define system_prompt, user_prompt, contexts[]
end
Model->>Proc: load action (system_prompt, user_prompt, contexts)
loop for each context
Proc->>Proc: read context.type / source / data / similarity_config
alt similarity_config present
Proc->>Proc: fetch top K similar docs, build similarity prompt entries
else slot/crud/action source
Proc->>Proc: build slot/crud/action-derived context payload
end
end
Proc->>LLM: send llmParams {system_prompt, user_prompt, history, similarity_prompts, payload}
alt success
LLM->>Proc: response
else error
LLM->>Proc: error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ 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 |
![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 (7)
kairon/shared/llm/processor.py (1)
215-243: user_prompt is collected but never used in messagesYou pass user_prompt through llm_params but __get_answer() ignores it. Include it in the user message so the new field actually affects completions.
Apply this diff:
@@ - instructions = kwargs.get('instructions', []) - instructions = '\n'.join(instructions) + instructions_list = kwargs.get('instructions', []) + instructions = '\n'.join(instructions_list) if instructions_list else '' + user_prompt = kwargs.get('user_prompt', '') @@ - messages.append({"role": "user", "content": f"{context} \n{instructions} \nQ: {query} \nA:"}) if instructions \ - else messages.append({"role": "user", "content": f"{context} \nQ: {query} \nA:"}) + parts = [] + if user_prompt: + parts.append(user_prompt) + parts.append(context) + if instructions: + parts.append(instructions) + parts.append(f"Q: {query} \nA:") + messages.append({"role": "user", "content": " \n".join(parts)})kairon/shared/chat/user_media.py (1)
463-473: PromptAction creation missing required fields (user_prompt, contexts) — likely save failureThe model requires contexts and user_prompt. Create a minimal static context and set user_prompt to avoid ValidationError and to activate the media-extraction guidance.
Apply these diffs:
@@ -from kairon.shared.actions.data_objects import Actions, PromptAction +from kairon.shared.actions.data_objects import Actions, PromptAction, Context @@ - prompt_action = PromptAction( - name=action_name, - system_prompt = f'{instructions[0]}', - dispatch_response=True, - process_media=True, - bot=bot, - status=True, - user="system", - ) + prompt_action = PromptAction( + name=action_name, + system_prompt=instructions[0], + user_prompt=instructions[0], + contexts=[ + Context( + name="Media Context", + data="Use the uploaded media (images, PDFs, docs) as primary context. If text is extracted, summarize it faithfully in markdown.", + type="user", + source="static", + is_enabled=True, + ) + ], + dispatch_response=True, + process_media=True, + bot=bot, + status=True, + user="system", + )kairon/shared/utils.py (1)
2187-2223: Clean up unused var and minor typo
- system_prompt_count is unused.
- Remove stray space in Utility.check_empty_string call.
Apply this diff:
@@ - system_prompt_count = 0 - history_prompt_count = 0 + history_prompt_count = 0 @@ - if Utility. check_empty_string(prompt.get("name")): + if Utility.check_empty_string(prompt.get("name")): raise exception_class("Name cannot be empty!")kairon/importer/validator/file_validator.py (1)
756-806: System prompt validation now fails for every prompt action
system_prompt_countis never incremented and the check for== 0still runs inside the loop. That means we append “System prompt is required” on the very first context (and on every subsequent one), so every prompt-action import now fails—even when a system prompt exists via the newsystem_promptfield. Please restore the increment and move the count checks after the loop (or drop them if the top-level field replaces this requirement).- system_prompt_count = 0 + system_prompt_count = 0 history_prompt_count = 0 for prompt in contexts: + if prompt.get('type') == 'system': + system_prompt_count += 1 if prompt.get('similarity_config') is not None: hyperparameters = prompt.get('similarity_config') for key, value in hyperparameters.items(): @@ - if system_prompt_count > 1: - error_list.append('Only one system prompt can be present') - if system_prompt_count == 0: - error_list.append('System prompt is required') - if history_prompt_count > 1: - error_list.append('Only one history source can be present') + if system_prompt_count > 1: + error_list.append('Only one system prompt can be present') + if system_prompt_count == 0: + error_list.append('System prompt is required') + if history_prompt_count > 1: + error_list.append('Only one history source can be present')kairon/shared/data/processor.py (2)
7946-7979: Null/KeyError risks in __validate_llm_prompts (crud_config/data).
- prompt["data"] may be missing → KeyError.
- prompt.get("crud_config") can be None → AttributeError on .get.
- Optional: validate CRUD query_source='slot' refers to an existing slot.
- def __validate_llm_prompts(self, contexts, bot: Text): - for prompt in contexts: - if prompt["source"] == "slot": - if not Utility.is_exist( - Slots, raise_error=False, name=prompt["data"], bot=bot, status=True - ): - raise AppException(f'Slot with name {prompt["data"]} not found!') - if prompt["source"] == "action": - if not ( + def __validate_llm_prompts(self, contexts, bot: Text): + for prompt in contexts or []: + source = (prompt or {}).get("source") + data = (prompt or {}).get("data") + if source == "slot": + if Utility.check_empty_string(data) or not Utility.is_exist( + Slots, raise_error=False, name=data, bot=bot, status=True + ): + raise AppException(f"Slot with name {data} not found!") + if source == "action": + if Utility.check_empty_string(data) or not ( Utility.is_exist( HttpActionConfig, raise_error=False, - action_name=prompt["data"], + action_name=data, bot=bot, status=True, ) or Utility.is_exist( GoogleSearchAction, raise_error=False, - name=prompt["data"], + name=data, bot=bot, status=True, ) ): - raise AppException(f'Action with name {prompt["data"]} not found!') + raise AppException(f"Action with name {data} not found!") - if prompt["source"] == "crud": - collections_list = DataProcessor.get_all_collections(bot) + if source == "crud": + collections_list = DataProcessor.get_all_collections(bot) existing_collections = {item['collection_name'] for item in collections_list} - missing_collections = [col for col in prompt.get('crud_config').get("collections",[]) if col not in existing_collections] + crud_cfg = (prompt or {}).get('crud_config') or {} + missing_collections = [col for col in (crud_cfg.get("collections") or []) if col not in existing_collections] if missing_collections: raise AppException(f'Collections not found: {missing_collections}') + # optional: if query_source is slot, validate slot name exists + if crud_cfg.get("query_source") == "slot": + qslot = crud_cfg.get("query") + if Utility.check_empty_string(qslot) or not Utility.is_exist( + Slots, raise_error=False, name=qslot, bot=bot, status=True + ): + raise AppException(f"Slot with name {qslot} not found for CRUD query.")
7931-7936: Normalize contexts to Context objects with field filtering to match edit_prompt_action behavior.The inconsistency is confirmed:
add_prompt_actionpasses raw dicts toPromptAction(**request_data)at line 7935, whileedit_prompt_actionexplicitly buildsContextobjects at lines 8006-8008. Raw dicts with unexpected fields (e.g., "instructions") will cause MongoEngine errors. The suggested fix correctly normalizes and filters to the seven allowed Context fields.- self.__validate_llm_prompts(request_data.get("contexts", []), bot) + self.__validate_llm_prompts(request_data.get("contexts", []), bot) + # normalize contexts to EmbeddedDocuments and filter allowed fields + allowed_ctx_fields = {"name","data","type","source","is_enabled","crud_config","similarity_config"} + request_data["contexts"] = [ + Context(**{k: v for k, v in (ctx or {}).items() if k in allowed_ctx_fields}) + for ctx in (request_data.get("contexts") or []) + ]kairon/shared/actions/data_objects.py (1)
819-837: Add validation forsimilarity_configplacement.The
similarity_configfield is added but lacks validation to ensure it's only provided whensourceisbot_content. Based on the logic inkairon/shared/data/data_models.py(lines 1138-1141),similarity_configshould be removed when the source is notbot_content.Apply this diff to add the missing validation:
if self.source == LlmPromptSource.bot_content.value and Utility.check_empty_string(self.data): self.data = "default" + + if self.source == LlmPromptSource.bot_content.value: + if self.similarity_config: + self.similarity_config.validate() + else: + if self.similarity_config: + raise ValidationError("similarity_config should only be provided when source is 'bot_content'")
🧹 Nitpick comments (9)
kairon/shared/llm/processor.py (1)
1-7: Safer similarity_context assembly: avoid raw repr, adjust label, and default pop
- Default pop to avoid KeyError when similarity_prompt missing.
- Replace “Instructions…” label with neutral “Context…”.
- Don’t dump Python list/dict repr into prompts; format as readable text (JSON or bullets).
Apply these diffs:
@@ -import os -import time +import os +import time +import json @@ - similarity_prompt = kwargs.pop('similarity_prompt') + similarity_prompt = kwargs.pop('similarity_prompt', []) @@ - if extracted_values: - similarity_context = f"Instructions on how to use {similarity_prompt_name}:\n{extracted_values}\n" - context_prompt = f"{context_prompt}\n{similarity_context}" + if extracted_values: + if isinstance(extracted_values, list): + flattened = "\n".join( + [ + f"- {val}" if isinstance(val, str) else f"- {json.dumps(val, ensure_ascii=False)}" + for val in extracted_values + ] + ) + else: + flattened = str(extracted_values) + similarity_context = f"Context from {similarity_prompt_name}:\n{flattened}\n" + context_prompt = f"{context_prompt}\n{similarity_context}"Also applies to: 408-437
kairon/shared/cognition/processor.py (1)
316-323: Tighten linked-action detection to bot_content contextsCurrent filter may match unrelated static/slot data named like the collection. Add source='bot_content' to avoid false positives.
Apply this diff:
- prompt_action = list(PromptAction.objects(bot=bot, contexts__data__iexact=collection)) + prompt_action = list( + PromptAction.objects( + bot=bot, + contexts__source="bot_content", + contexts__data__iexact=collection, + ) + )kairon/shared/data/data_validation.py (2)
92-103: Guard contexts access and emit clear error when missingAvoid KeyError and provide actionable feedback if contexts are absent.
Apply this diff:
- llm_prompts_errors = DataValidation.validate_llm_prompts(data['contexts']) - data_error.extend(llm_prompts_errors) + contexts = data.get('contexts') or [] + if not contexts: + data_error.append('contexts are required') + else: + llm_prompts_errors = DataValidation.validate_llm_prompts(contexts) + data_error.extend(llm_prompts_errors)
135-188: Align contexts validation with new model; fix f-string and add type checks
- Don’t require a system context now that system_prompt is top-level.
- Validate similarity_config type before iterating.
- Fix extraneous f-string.
Apply this diff:
- system_prompt_count = 0 - history_prompt_count = 0 + history_prompt_count = 0 @@ - if prompt.get('similarity_config') is not None: - hyperparameters = prompt.get('similarity_config') - for key, value in hyperparameters.items(): + cfg = prompt.get('similarity_config') + if cfg is not None: + if not isinstance(cfg, dict): + error_list.append("similarity_config must be an object") + cfg = {} + for key, value in cfg.items(): @@ - error_list.append( - f"similarity_threshold should be within 0.3 and 1.0 and of type int or float!") + error_list.append( + "similarity_threshold should be within 0.3 and 1.0 and of type int or float!") @@ - if prompt.get('type') == 'system': - system_prompt_count += 1 - elif prompt.get('source') == 'history': + if prompt.get('source') == 'history': history_prompt_count += 1 @@ - if prompt.get('type') == 'system' and prompt.get('source') != 'static': - error_list.append('System prompt must have static source') + # system prompt is provided via top-level system_prompt; no system context checks @@ - if system_prompt_count > 1: - error_list.append('Only one system prompt can be present') - if system_prompt_count == 0: - error_list.append('System prompt is required') + # no system context required/allowed checks here if history_prompt_count > 1: error_list.append('Only one history source can be present')tests/integration_test/services_test.py (3)
12408-12408: Remove commented-outinstructionsfields.The commented-out
instructionsfields appear to be leftover from the refactoring where instructions were removed from the context structure. These should be cleaned up.For example:
{ "name": "Similarity Prompt", - # 'instructions': 'Answer question based on the context above, if answer is not in the context go check previous logs.', 'type': 'user', 'source': 'bot_content',Also applies to: 12412-12412, 12417-12417, 13143-13143, 13151-13151, 13189-13189, 13197-13197, 13243-13243, 13259-13259, 13292-13292, 13300-13300, 13339-13339, 13347-13347, 13355-13355, 13398-13398, 13406-13406, 13414-13414, 13453-13453, 13461-13461, 13469-13469, 13513-13513, 13521-13521, 13529-13529, 13749-13749, 13757-13757, 13800-13800, 13808-13808, 13816-13816, 13863-13863, 13871-13871, 13879-13879, 13924-13924, 13932-13932, 13940-13940, 13982-13982, 13990-13990, 13998-13998, 14031-14031, 14039-14039, 14047-14047, 14081-14081, 14089-14089, 14097-14097, 14132-14132, 14140-14140, 14174-14174, 14182-14182, 14218-14218, 14226-14226, 14266-14266, 14274-14274, 14321-14321, 14329-14329, 14337-14337, 14400-14400, 14408-14408, 14416-14416, 14451-14451, 14459-14459, 14467-14467, 14530-14530, 14538-14538, 14546-14546, 14598-14598, 14602-14602, 14607-14607, 14662-14662, 14670-14670, 14678-14678, 14732-14732, 14736-14736, 14741-14741, 14789-14789, 14797-14797, 14805-14805, 14852-14852, 14902-14902, 14953-14953, 15003-15003, 15056-15056, 36540-36540, 36548-36548, 36556-36556, 36778-36778, 36786-36786, 36794-36794
13131-13132: Inconsistent formatting: extra blank line after contexts opening bracket.There are extra blank lines after the
"contexts": [opening bracket in many test cases. While not a functional issue, this creates inconsistent formatting across the test file.Consider removing these extra blank lines for consistency:
- "contexts": [ - + "contexts": [ {Also applies to: 13184-13185, 13335-13336, 13394-13395, 13449-13450, 13508-13509, 13620-13621, 13675-13676, 13734-13735, 13859-13860, 13920-13921, 13978-13979, 14027-14028, 14076-14077, 14127-14128, 14170-14171, 14214-14215, 14262-14263, 14317-14318, 14396-14397, 14447-14448, 14526-14527, 14658-14659, 14785-14786, 14849-14850, 14899-14900, 14950-14951, 15000-15001, 15053-15054, 15370-15371, 15407-15422, 36536-36537, 36774-36775
14617-14622: Remove commented-out code.The commented-out
print(actual)line should be removed.- # print(actual) response = client.post(kairon/shared/data/processor.py (1)
380-383: Slot mapping misses CRUD query slot usage.If a context has source='crud' with crud_config.query_source='slot', the slot is used but won’t be detected here. Add a check so get_slot_mapped_actions reports correct dependencies.
- contexts = config.get("contexts", []) - for prompt in contexts: + contexts = config.get("contexts", []) + for prompt in contexts: if prompt.get('source') == "slot" and prompt.get('data') == slot: return True + # also consider CRUD contexts where the query is sourced from a slot + if prompt.get('source') == "crud": + crud_cfg = prompt.get('crud_config') or {} + if crud_cfg.get('query_source') == 'slot' and crud_cfg.get('query') == slot: + return Truekairon/shared/actions/data_objects.py (1)
857-857: Consider markingcontextsas required.The
contextsfield is validated as required in thevalidatemethod (line 877), but it's not marked asrequired=Trueat the schema level. According to the AI summary, the previousllm_promptsfield was marked asrequired=True. For consistency and clarity, consider marking this field as required.Apply this diff:
- contexts = EmbeddedDocumentListField(Context) + contexts = EmbeddedDocumentListField(Context, required=True)This change ensures the requirement is enforced at both the schema and validation levels.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
kairon/actions/definitions/prompt.py(2 hunks)kairon/importer/validator/file_validator.py(3 hunks)kairon/shared/actions/data_objects.py(4 hunks)kairon/shared/chat/user_media.py(1 hunks)kairon/shared/cognition/processor.py(1 hunks)kairon/shared/data/data_models.py(6 hunks)kairon/shared/data/data_validation.py(3 hunks)kairon/shared/data/processor.py(6 hunks)kairon/shared/llm/processor.py(2 hunks)kairon/shared/utils.py(3 hunks)template/use-cases/Hi-Hello-GPT/actions.yml(1 hunks)tests/integration_test/services_test.py(67 hunks)tests/testing_data/actions/actions.yml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
kairon/shared/utils.py (1)
kairon/shared/models.py (1)
LlmPromptSource(96-103)
tests/integration_test/services_test.py (3)
kairon/shared/data/data_objects.py (1)
BotSettings(903-949)kairon/shared/utils.py (3)
Utility(93-2341)get_default_llm_hyperparameters(2138-2139)get_llm_hyperparameters(2142-2148)kairon/shared/actions/data_objects.py (3)
PromptAction(847-887)UserQuestion(839-842)SetSlotsFromResponse(96-102)
kairon/shared/cognition/processor.py (1)
kairon/shared/actions/data_objects.py (1)
PromptAction(847-887)
kairon/shared/data/data_models.py (3)
kairon/shared/actions/data_objects.py (1)
Context(795-836)kairon/shared/models.py (1)
LlmPromptSource(96-103)kairon/shared/utils.py (3)
Utility(93-2341)check_empty_string(148-160)validate_llm_hyperparameters(2151-2163)
kairon/actions/definitions/prompt.py (2)
kairon/shared/models.py (2)
LlmPromptType(106-109)LlmPromptSource(96-103)kairon/shared/actions/utils.py (1)
prepare_bot_responses(263-281)
kairon/shared/actions/data_objects.py (2)
kairon/shared/data/data_models.py (1)
Context(1105-1144)kairon/shared/utils.py (2)
Utility(93-2341)validate_kairon_faq_llm_prompts(2187-2223)
kairon/shared/data/data_validation.py (1)
kairon/shared/data/data_models.py (1)
validate_llm_prompts(1189-1195)
kairon/shared/data/processor.py (3)
kairon/shared/actions/data_objects.py (3)
Context(795-836)PromptAction(847-887)UserQuestion(839-842)kairon/shared/data/data_models.py (1)
Context(1105-1144)kairon/shared/models.py (1)
LlmPromptSource(96-103)
🪛 Ruff (0.14.3)
kairon/shared/utils.py
2188-2188: Local variable system_prompt_count is assigned to but never used
Remove assignment to unused variable system_prompt_count
(F841)
kairon/shared/data/data_models.py
1209-1209: Avoid specifying long messages outside the exception class
(TRY003)
1213-1213: Avoid specifying long messages outside the exception class
(TRY003)
1216-1216: Prefer TypeError exception for invalid type
(TRY004)
1216-1216: Avoid specifying long messages outside the exception class
(TRY003)
kairon/shared/actions/data_objects.py
878-878: Avoid specifying long messages outside the exception class
(TRY003)
kairon/shared/data/data_validation.py
147-147: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Python CI
- GitHub Check: Analyze (python)
🔇 Additional comments (13)
template/use-cases/Hi-Hello-GPT/actions.yml (1)
28-37: LGTM on migrating to system_prompt/user_prompt + contextsTemplate matches the new config shape and defaults are reasonable.
tests/testing_data/actions/actions.yml (1)
266-278: Confirm no duplicate system instructions at runtimeYou now have top-level system_prompt and also a system context here. Ensure the system context isn’t concatenated into messages to avoid duplication. If unnecessary, consider removing it from tests for clarity.
Also applies to: 291-302
kairon/actions/definitions/prompt.py (2)
155-157: user_prompt passthrough is fine; ensure the runtime uses itYou correctly add user_prompt to llmParams. With the proposed LLMProcessor.__get_answer change, it will be included in the user message.
Also applies to: 235-237
158-175: Similarity prompt config read looks goodUsing similarity_config with sane defaults (top_results=10, threshold=0.7) aligns with the refactor goals.
tests/integration_test/services_test.py (4)
15370-15392: Test setup uses direct database insertion with contexts.These test setup sections directly create
PromptActionobjects with the newcontextsstructure andsystem_prompt/user_promptfields. The structure looks correct and consistent with the data model changes.Also applies to: 15395-15405, 15407-15445
14845-14868: New test coverage for 'crud' source type.These tests validate the new 'crud' source type for contexts, including validation of query formats (JSON object, string query, slot-based query, and invalid JSON). This provides good coverage for the new functionality.
Also applies to: 14895-14918, 14946-14969, 14996-15019, 15049-15072
13445-13500: Test is incorrect—wrong data structure being tested.The test passes a list for
system_promptfield (which expectsstrper the Pydantic model), but expects an error about thecontextsarray. When Pydantic validatessystem_promptas a list, it will reject at the field level with error location["body", "system_prompt"], not["body", "contexts"].The error message "Only one system prompt can be present!" refers to validation of multiple system-type prompts within the
contextsarray, not thesystem_promptfield type. To properly test that validation, pass multiple context objects with"type": "system", not a list value for thesystem_promptfield. Either fix the test data to match the expected error location/message, or adjust expectations to match what a list value forsystem_promptactually produces.Likely an incorrect or invalid review comment.
13224-13225: Error message validations confirmed—no issues found.All error message assertions in the integration tests are correctly aligned with backend Pydantic validators in
kairon/shared/data/data_models.py:
- similarity_threshold (line 13224-13225): Test expects
'similarity_threshold should be within 0.3 and 1'— matches validator at line 1093 ✓- top_results (line 13278-13279): Test expects
'top_results should not be greater than 30'— matches validator at line 1095 ✓- system_prompt (line 13435-13436): Test expects
'System prompt cannot be empty!'— matches dynamic error at line 1213 ✓- user_prompt (line 13552-13553): Test expects
'User prompt cannot be empty!'— matches dynamic error at line 1213 ✓The error messages are dynamically generated at line 1213 using
f"{field.name.replace('_', ' ').capitalize()} cannot be empty!", correctly producing the expected formats.kairon/shared/data/processor.py (3)
83-83: Import of Context looks good.Aligns this module with the new Context model.
704-706: Good guard to prevent deleting actions referenced by PromptAction contexts.The contexts__source/contexts__data filter matches the new schema.
7998-7999: LGTM on switching validation to contexts.Call site updated correctly.
kairon/shared/actions/data_objects.py (2)
862-862: LGTM!The
process_mediafield addition is straightforward with a sensible default value.
858-859: LGTM!The addition of
system_promptanduser_promptas required fields provides a clearer structure compared to the previousinstructionsfield.
| if not self.contexts: | ||
| raise ValidationError("llm_prompts are required!") |
There was a problem hiding this comment.
Update error message to reflect the new field name.
The error message references the old field name llm_prompts, but the field has been renamed to contexts. This inconsistency could confuse users.
Apply this diff to fix the error message:
if not self.contexts:
- raise ValidationError("llm_prompts are required!")
+ raise ValidationError("contexts are required!")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not self.contexts: | |
| raise ValidationError("llm_prompts are required!") | |
| if not self.contexts: | |
| raise ValidationError("contexts are required!") |
🧰 Tools
🪛 Ruff (0.14.3)
878-878: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In kairon/shared/actions/data_objects.py around lines 877 to 878, the
ValidationError message still references the old field name "llm_prompts";
update the exception text to reference the renamed field "contexts" instead
(e.g., raise ValidationError("contexts are required!")) so the error message
matches the current field name.
| action.contexts = [ | ||
| Context(**prompt) for prompt in request_data.get("contexts", []) | ||
| ] | ||
| action.instructions = request_data.get("instructions", []) | ||
| action.system_prompt = request_data.get("system_prompt", []) | ||
| action.user_prompt = request_data.get("user_prompt", []) | ||
| action.set_slots = request_data.get("set_slots", []) |
There was a problem hiding this comment.
Wrong defaults and potential FieldDoesNotExist when updating contexts; fix type of prompts.
- action.system_prompt/user_prompt default to [] (should be str). Use presence checks.
- Context(**prompt) may break on unknown keys. Filter to allowed fields, as in creation.
- action.contexts = [
- Context(**prompt) for prompt in request_data.get("contexts", [])
- ]
- action.system_prompt = request_data.get("system_prompt", [])
- action.user_prompt = request_data.get("user_prompt", [])
+ allowed_ctx_fields = {"name","data","type","source","is_enabled","crud_config","similarity_config"}
+ if "contexts" in request_data:
+ action.contexts = [
+ Context(**{k: v for k, v in (prompt or {}).items() if k in allowed_ctx_fields})
+ for prompt in (request_data.get("contexts") or [])
+ ]
+ if "system_prompt" in request_data:
+ action.system_prompt = request_data["system_prompt"]
+ if "user_prompt" in request_data:
+ action.user_prompt = request_data["user_prompt"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| action.contexts = [ | |
| Context(**prompt) for prompt in request_data.get("contexts", []) | |
| ] | |
| action.instructions = request_data.get("instructions", []) | |
| action.system_prompt = request_data.get("system_prompt", []) | |
| action.user_prompt = request_data.get("user_prompt", []) | |
| action.set_slots = request_data.get("set_slots", []) | |
| allowed_ctx_fields = {"name","data","type","source","is_enabled","crud_config","similarity_config"} | |
| if "contexts" in request_data: | |
| action.contexts = [ | |
| Context(**{k: v for k, v in (prompt or {}).items() if k in allowed_ctx_fields}) | |
| for prompt in (request_data.get("contexts") or []) | |
| ] | |
| if "system_prompt" in request_data: | |
| action.system_prompt = request_data["system_prompt"] | |
| if "user_prompt" in request_data: | |
| action.user_prompt = request_data["user_prompt"] | |
| action.set_slots = request_data.get("set_slots", []) |
🤖 Prompt for AI Agents
In kairon/shared/data/processor.py around lines 8006–8011, the code assigns
defaults incorrectly and risks FieldDoesNotExist when updating contexts; change
system_prompt and user_prompt to only be set when present (use request_data.get
and check for None/exists) so they remain strings (or None) rather than
defaulting to [] and keep set_slots as is; when constructing Context objects
from request_data["contexts"], filter each prompt dict to only the allowed
Context fields (use the same allowed_fields list used in creation or Context
model field names) before calling Context(**filtered_prompt) to avoid
unknown-key errors.
| }, | ||
| { | ||
| "name": "Query Prompt", | ||
| "data": "If there is no specific query, assume that user is aking about java programming.", |
There was a problem hiding this comment.
Fix repeated typo across multiple test cases.
The word "aking" should be "asking" in multiple test data strings.
This typo appears in approximately 20 locations throughout the test file. Apply similar fixes to all occurrences:
- 'data': 'If there is no specific query, assume that user is aking about java programming.',
+ 'data': 'If there is no specific query, assume that user is asking about java programming.',Also applies to: 13258-13258, 13354-13354, 13468-13468, 13528-13528, 13587-13587, 13691-13691, 13757-13757, 13815-13815, 13877-13877, 14139-14139, 14181-14181, 14336-14336, 14415-14415, 14466-14466, 14545-14545, 14606-14606, 14677-14677, 14740-14740, 14804-14804
| bot_settings = BotSettings.objects(bot=pytest.bot).get() | ||
| bot_settings.llm_settings.enable_faq = True | ||
| bot_settings.save() | ||
| print(bot_settings.to_json()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug print statements.
Debug print statements should be removed from test code before merging.
Apply this diff to remove the print statements:
- print(bot_settings.to_json())- print(actual)Also applies to: 13483-13483, 13505-13505, 13543-13543, 14635-14635
🤖 Prompt for AI Agents
In tests/integration_test/services_test.py around lines 13390, 13483, 13505,
13543, and 14635 there are leftover debug print() calls (e.g.,
print(bot_settings.to_json())) that must be removed; edit each listed line to
delete the print statement so tests don't emit debug output and commit the
changes.
| bot_settings = BotSettings.objects(bot=pytest.bot).get() | ||
| print(bot_settings.to_json()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove duplicate bot_settings retrieval.
The bot_settings object is retrieved and printed twice consecutively, which is redundant.
Apply this diff:
}
- bot_settings = BotSettings.objects(bot=pytest.bot).get()
- print(bot_settings.to_json())
response = client.post(Also applies to: 13542-13543
🤖 Prompt for AI Agents
In tests/integration_test/services_test.py around lines 13482-13483 and
13542-13543, the BotSettings.objects(bot=pytest.bot).get() call is duplicated
and printed twice; remove the redundant retrieval and print so bot_settings is
retrieved once and reused. Replace the second duplicate lines in each location
with nothing (delete them) or reference the existing bot_settings variable
instead of re-querying, ensuring no behavioral change to the test.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/unit_test/action/action_test.py (2)
4760-4766: Use ContextDoc for MongoEngine validationsAfter aliasing, update MongoEngine-based tests to use ContextDoc.
Apply this diff:
- prompt = Context( + prompt = ContextDoc( @@ - prompt = Context( + prompt = ContextDoc(Also applies to: 4774-4785
4802-4809: Use ContextModel for Pydantic validationsSwap constructor calls to the aliased Pydantic class.
Apply this diff:
- Context( + ContextModel( @@ - Context( + ContextModel( @@ - Context( + ContextModel(Also applies to: 4821-4829, 4841-4845
♻️ Duplicate comments (2)
tests/integration_test/services_test.py (2)
13380-13390: Remove leftover debug print statements.The
print(...)calls (for example, Lines 13380, 13467, 13524, and 14581) were already flagged and still need to be dropped so the integration tests stay clean.Apply this diff pattern:
- print(bot_settings.to_json()) … - print(actual)Also applies to: 13466-13474, 13524-13532, 14580-14582
12416-12419: Fix repeated “aking” typo in prompt data.The sample prompt text still says “aking” instead of “asking”. Please update this string everywhere it appears so the test fixtures read correctly.
Apply this representative diff (repeat for the other occurrences):
- "data": "If there is no specific query, assume that user is aking about java programming.", + "data": "If there is no specific query, assume that user is asking about java programming.",Also applies to: 13148-13152, 13252-13256, 13344-13348, 13400-13404, 13502-13512, 13734-13738, 13790-13794, 13850-13854, 13908-13912, 13964-13968, 14008-14012, 14056-14060, 14136-14142, 14226-14230, 14284-14288, 14363-14368, 14412-14416, 14622-14627, 14749-14754
🧹 Nitpick comments (1)
tests/unit_test/action/action_test.py (1)
4794-4795: Remove duplicate local imports of Pydantic ContextOnce aliased at the top, these re-imports are unnecessary and risk confusion. Use ContextModel directly.
Apply this diff:
-from kairon.shared.data.data_models import Context +# use ContextModel from top-level import @@ -from kairon.shared.data.data_models import Context +# use ContextModel from top-level import @@ -from kairon.shared.data.data_models import Context +# use ContextModel from top-level importAlso applies to: 4812-4814, 4831-4833
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
kairon/shared/data/data_validation.py(3 hunks)tests/integration_test/services_test.py(64 hunks)tests/unit_test/action/action_test.py(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- kairon/shared/data/data_validation.py
🧰 Additional context used
🧬 Code graph analysis (2)
tests/integration_test/services_test.py (2)
kairon/shared/data/data_objects.py (1)
BotSettings(903-949)kairon/shared/utils.py (3)
Utility(93-2341)get_default_llm_hyperparameters(2138-2139)get_llm_hyperparameters(2142-2148)
tests/unit_test/action/action_test.py (2)
kairon/shared/data/data_models.py (2)
Context(1105-1144)CrudConfigRequest(1099-1103)kairon/shared/actions/data_objects.py (3)
Context(795-836)CrudConfig(772-793)PromptAction(847-887)
🪛 Ruff (0.14.3)
tests/unit_test/action/action_test.py
49-49: Redefinition of unused Context from line 12
(F811)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (python)
- GitHub Check: Python CI
🔇 Additional comments (4)
tests/unit_test/action/action_test.py (4)
3311-3323: LGTM: contexts + system/user prompts setupThe new contexts array and separate system_prompt/user_prompt align with the updated model. Save flow looks correct.
3338-3351: LGTM: assertions match new payload shapeAssertions cover contexts with similarity_config and the new system_prompt/user_prompt fields.
Consider adding one negative assertion to ensure no legacy llm_prompts key exists in the payload.
4701-4712: LGTM: second prompt action uses new schemaContexts, system_prompt, and user_prompt are correctly populated before retrieval.
4724-4734: LGTM: config assertions reflect new fieldsConfig check validates contexts and both prompt fields; consistent with data model changes.
| from kairon.exceptions import AppException | ||
| from kairon.shared.admin.data_objects import LLMSecret | ||
| from kairon.shared.data.data_models import LlmPromptRequest, CrudConfigRequest | ||
| from kairon.shared.data.data_models import Context, CrudConfigRequest |
There was a problem hiding this comment.
Fix Context import collision (ruff F811) by aliasing both types
Two different Context classes are imported, the second redefines the first. Alias them to avoid shadowing and confusion across tests.
Apply this diff:
-from kairon.shared.data.data_models import Context, CrudConfigRequest
+from kairon.shared.data.data_models import Context as ContextModel, CrudConfigRequest
@@
-from kairon.shared.actions.data_objects import HttpActionRequestBody, HttpActionConfig, ActionServerLogs, SlotSetAction, \
+from kairon.shared.actions.data_objects import HttpActionRequestBody, HttpActionConfig, ActionServerLogs, SlotSetAction, \
Actions, FormValidationAction, EmailActionConfig, GoogleSearchAction, JiraAction, ZendeskAction, \
PipedriveLeadsAction, SetSlots, HubspotFormsAction, HttpActionResponse, CustomActionRequestParameters, \
KaironTwoStageFallbackAction, SetSlotsFromResponse, PromptAction, PyscriptActionConfig, WebSearchAction, \
- CustomActionParameters, ParallelActionConfig, DatabaseAction, CrudConfig, Context
+ CustomActionParameters, ParallelActionConfig, DatabaseAction, CrudConfig, Context as ContextDocAlso applies to: 49-49
🤖 Prompt for AI Agents
In tests/unit_test/action/action_test.py around lines 12 and 49, two different
Context classes are being imported and the second import shadows the first (ruff
F811); update the imports to alias each Context to distinct names (e.g.,
ContextModel and ContextDto or similar) and then update all usages in this file
to use the new aliases so there is no name collision.
| contexts = [ | ||
| { | ||
| 'source': 'static', 'is_enabled': True, 'collections': [], 'result_limit': 10, 'query': {}}, | ||
| crud_prompt | ||
| ] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix malformed static context in request (missing name/type; CRUD-only keys present)
The first entry under contexts lacks required fields (name, type) and includes CRUD-only keys. It will fail validation before your missing-collection check.
Apply this diff to define a valid static context (or remove it if not needed):
- contexts = [
- {
- 'source': 'static', 'is_enabled': True, 'collections': [], 'result_limit': 10, 'query': {}},
- crud_prompt
- ]
+ contexts = [
+ {'name': 'Static Prompt', 'type': 'user', 'source': 'static', 'is_enabled': True},
+ crud_prompt
+ ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| contexts = [ | |
| { | |
| 'source': 'static', 'is_enabled': True, 'collections': [], 'result_limit': 10, 'query': {}}, | |
| crud_prompt | |
| ] | |
| contexts = [ | |
| {'name': 'Static Prompt', 'type': 'user', 'source': 'static', 'is_enabled': True}, | |
| crud_prompt | |
| ] |
🤖 Prompt for AI Agents
In tests/unit_test/action/action_test.py around lines 4876 to 4880, the first
contexts entry is malformed: it lacks required fields (name and type) and
contains CRUD-only keys (collections, result_limit, query) which cause
validation to fail before the missing-collection assertion runs; replace that
entry with a valid static context (or remove it) by providing required fields
(e.g., name and type='static') and remove CRUD-only keys or move them into the
appropriate CRUD context (or set the static context to an empty/static payload
that passes schema validation) so the test proceeds to the intended
missing-collection check.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/integration_test/action_service_test.py(47 hunks)tests/testing_data/actions/actions.yml(2 hunks)tests/testing_data/use-cases/Hi-Hello-GPT/actions.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/integration_test/action_service_test.py (1)
kairon/shared/actions/data_objects.py (2)
PromptAction(847-887)UserQuestion(839-842)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (python)
- GitHub Check: Python CI
🔇 Additional comments (2)
tests/testing_data/actions/actions.yml (1)
266-295: Verify the "instructions" field consistency across context definitions.The first context (lines 267-272) includes an
instructionsfield, but the second context (lines 289-293) does not. Ifinstructionsis an optional field in the Context model, this is acceptable; otherwise, this could indicate incomplete test data coverage.Can you confirm whether:
- The
instructionsfield is optional in the Context class?- If optional, both test contexts should be equally valid with or without it?
- Whether context type (
queryvsuser) determines whetherinstructionsshould be present?Additionally, verify that test cases cover both scenarios (with and without
instructions) to ensure the prompt assembly logic handles both correctly.tests/testing_data/use-cases/Hi-Hello-GPT/actions.yml (1)
28-37: Verify slot-based context data reference and multi-line prompt formatting.The context uses
source: slotwithdata: google_search_result. Ensure this slot name is properly defined in the bot's slot configuration and is resolved correctly during prompt assembly.Additionally, verify that the multi-line
system_prompt(lines 35-36) is parsed correctly by the YAML parser and that whitespace/line breaks are handled as intended in the LLM request payload.Can you confirm:
- That
google_search_resultis a valid slot name defined elsewhere in the test fixtures?- That the prompt payload correctly includes the multi-line system prompt without unintended whitespace artifacts?
| user_prompt="Answer in a short manner. Keep it simple.", | ||
| system_prompt= "You are a personal assistant.", |
There was a problem hiding this comment.
Remove trailing commas that turn prompts into tuples
Each user_prompt / system_prompt assignment ends with a trailing comma, so Python stores them as one-element tuples instead of strings. PromptAction expects StringField values, and the tuple breaks validation (or later string operations). Clean this up everywhere this pattern appears in the file—for example:
- user_prompt="Answer in a short manner. Keep it simple.",
- system_prompt= "You are a personal assistant.",
+ user_prompt="Answer in a short manner. Keep it simple.",
+ system_prompt= "You are a personal assistant."(Repeat for every other user_prompt / system_prompt assignment that currently ends with a comma.)
Also applies to: 12470-12471, 12701-12702, 12795-12796, 12892-12893
🤖 Prompt for AI Agents
In tests/integration_test/action_service_test.py around lines 12381-12382 (and
also clean up at 12470-12471, 12701-12702, 12795-12796, 12892-12893), remove the
trailing commas after user_prompt and system_prompt assignments so the values
are plain strings instead of one-element tuples; update each occurrence by
deleting the comma at the end of the assignment line so PromptAction receives a
string and validation/string operations work correctly.
1 participant
Summary by CodeRabbit
New Features
crudas a context source.system_promptanduser_promptfields.similarity_configper context for similarity controls.Refactor
contextsinstead of the prior prompt structure.