Release 1.0.0 - #2
Merged
kiranharidas187 merged 12 commits intoJun 1, 2026
Merged
Conversation
… and made it configurable from admin panel
… and chunk persistence - Add download_file handler in CommonResponseHandler generating both PDF (Jinja2+Gotenberg) and DOCX (python-docx) and saving both URLs to CompanyChat.other_params and WebSocket extra_content - Add render_template_to_pdf (Flow/PDFTemplates lookup, Jinja2 context with args/constants/profile/sources) and create_docx_from_args (structured MIP sections + References) - Add sanitize_filename with extension param and MediaTypeChoices.DOCX - Wire finalized_sources through respond_to_user tool: save to ChatSession.other_params on each call, inject into system prompt each turn, use at download_file time for References section in both formats - Fix chunk accumulation in tool loop: retrieved_chunks now a list extended across all iterations instead of overwritten; pass through to process_response via extra dict key _retrieved_chunks - Fix chunks not saved to DB: serialize as JSON string (TextField was storing Python repr); add _safe_json_dumps helper with silent fallback - Append Sources used block to assistant messages in chat history when chunks are present, enabling LLM to track which sources backed accepted content - Add citation chunk extraction for streaming path: call_llm_gateway_stream yields 4-tuple with citations; _handle_gateway_stream merges KB + citation chunks before save - Fix _handle_freeflow_function_call routing: explicit _freeflow_action_tools / _json_message_tools dispatch in process_response - Fix respond_to_user message extraction: fall back to response key when message key absent; extract quick_reply_chips - Format KB search results with Source: [Title](url) header so LLM can cite them
Skip web-search retry when LLM returns a non-empty response — the tokens were already streamed to the client, so retrying caused a second complete message to appear. For download_file: when the LLM streams text before calling the tool, signal this via _text_streamed_to_ws so _handle_freeflow_function_call sends only a stop chunk with the download URLs instead of repeating the bot_message through translate_and_send_message.
- Extract usage (input/output tokens, cost) from gateway responses for both streaming and non-streaming paths - Accumulate full turn cost across tool-loop iterations (KB search + retries) so CompanyChat.other_params reflects the total turn spend, not just the final call - Update ChatSession.other_params with running totals after every LLM call - Fix CompanyChat query to filter by sender__id=1 (bot profile) instead of the non-existent initiated_by field - Yield usage data from stream finish event so streaming path tracks cost the same way as non-stream - Normalize respond_to_user tool args (quick_reply_chips, finalized_sources) that Llama returns as JSON strings instead of arrays - Embed hyperlinks in DOCX references section instead of raw URL text
📝 WalkthroughWalkthroughThis PR integrates an external LLM gateway service to replace provider-specific OpenAI/Bedrock handling, adds vector knowledge-base and web-search query capabilities to CompanyBot, introduces PDF template and DOCX generation with hyperlinks, and refactors response handlers to execute tools iteratively through the gateway. It also updates Celery tasks to support message appending, converts configuration to environment-driven values, and disables Django admin interfaces. ChangesLLM Gateway Integration & Schema Updates
Media Generation & Chat Utilities
Persistence & Configuration
🎯 4 (Complex) | ⏱️ ~75 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 Warning |
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
chatbot/admin/story_admin.py (1)
1-184:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDelete the commented-out
story_admin.py(or explicitly re-enable admin functionality)
chatbot/admin/__init__.pyimportsfrom .story_admin import *, so withchatbot/admin/story_admin.pyfully commented out,StoryAdmin/StoryTranslationAdminregistrations and the admin-only functionality (export routes, PDF hooks insave_model, post-processing route) will not be available.- If removal is intended, delete
chatbot/admin/story_admin.py; if it’s temporary, replace the commented file with a minimal stub and add a clear TODO/expiry note stating when it will be restored.🤖 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 `@chatbot/admin/story_admin.py` around lines 1 - 184, The file chat_admin/story_admin.py is fully commented out but is imported by chatbot/admin/__init__.py causing admin registrations and routes (StoryAdmin, StoryTranslationAdmin, export_stories_view, save_model PDF hooks, PostProcessingView route) to be missing; either delete story_admin.py if removal is intentional, or restore minimal admin functionality by re-enabling the StoryAdmin and StoryTranslationAdmin registrations (or create a minimal stub that defines StoryAdmin and StoryTranslationAdmin as no-op admin.ModelAdmin subclasses or safely guards the import) and add a clear TODO comment with an expiry date; ensure the symbols StoryAdmin, StoryTranslationAdmin, export_stories_view (or PostProcessingView route), and save_model logic are present or stubbed so chatbot/admin/__init__.py import no longer breaks.chatbot/models/company_models.py (1)
73-94: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRemove the duplicate
provider/provider_keysfield definitions.
providerandprovider_keysare each declared twice (Lines 73-76 and again Lines 87-94). Since these are class attributes, the second definitions silently shadow the first, making Lines 73-76 dead code. This is confusing and a maintenance hazard — keep only the intended definition (the one withdefault=LLMProvider.BEDROCKand the updatedhelp_text).♻️ Proposed cleanup
- provider = models.CharField( - max_length=100, choices=LLMProvider.choices, default=LLMProvider.OPENAI) - provider_keys = models.TextField( - default="", max_length=1000, null=False, blank=True) bot_temperature = models.FloatField(🤖 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 `@chatbot/models/company_models.py` around lines 73 - 94, The model defines provider and provider_keys twice causing the first declarations to be shadowed; remove the duplicate early definitions and keep the intended variants that use default=LLMProvider.BEDROCK and include the help_text. Specifically, delete the initial provider = models.CharField(...) and provider_keys = models.TextField(...) occurrences and ensure only the provider and provider_keys fields with default=LLMProvider.BEDROCK and the updated help_text remain in the model (referencing the provider and provider_keys attributes and LLMProvider choices).
🧹 Nitpick comments (10)
chatbot/admin/theme_admin.py (1)
1-40: ⚡ Quick winDelete the file rather than commenting out all code.
Same concern as
story_admin.py— if Theme admin is being permanently disabled, remove the file entirely instead of leaving commented code. If temporary, document why.🤖 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 `@chatbot/admin/theme_admin.py` around lines 1 - 40, The file contains a fully commented-out Django admin class (ThemeAdmin) and related imports (Theme, ThemeType, get_form, changeform_view) — remove the file entirely if the Theme admin is permanently disabled; if it's temporary, restore a minimal, documented placeholder (keep a short module-level comment explaining why it's disabled and a TODO with an expected re-enable date or issue ref) or re-enable by uncommenting and ensuring ThemeAdmin, get_form and changeform_view are correct; commit the deletion or the documented placeholder so the repo no longer contains a whole commented-out implementation.shikshalokam_mohini/settings.py (1)
54-54: 💤 Low valueOptional: fail fast with a clear message when
DJANGO_SECRET_KEYis missing.
os.getenv('DJANGO_SECRET_KEY')returnsNonewhen the variable is unset. Django will eventually raise, but the error surfaces late and indirectly (andSIGNING_KEY = SECRET_KEYon Line 372 would silently becomeNonefor JWT). A guard makes misconfiguration obvious at startup.♻️ Proposed guard
-SECRET_KEY = os.getenv('DJANGO_SECRET_KEY') +SECRET_KEY = os.getenv('DJANGO_SECRET_KEY') +if not SECRET_KEY: + raise ValueError("DJANGO_SECRET_KEY environment variable is required")🤖 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 `@shikshalokam_mohini/settings.py` at line 54, SECRET_KEY is set from os.getenv('DJANGO_SECRET_KEY') which can be None; add a fail-fast guard in settings so that if SECRET_KEY is falsy you raise a clear exception at import time. Locate the SECRET_KEY assignment and follow-up use of SIGNING_KEY (SIGNING_KEY = SECRET_KEY) and replace with logic that validates SECRET_KEY (e.g., check SECRET_KEY after assignment) and raise a RuntimeError or ImproperlyConfigured with a clear message instructing to set DJANGO_SECRET_KEY when missing.chatbot/utils/media_preview/media_creation.py (1)
291-295: 💤 Low valueRemove debug
print()statements.Lines 291, 293, and 295 use
print()for diagnostics that are already covered by thelogger.infocall on Line 297. Drop the prints to avoid noisy stdout in production.🤖 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 `@chatbot/utils/media_preview/media_creation.py` around lines 291 - 295, Remove the three debug print() calls around flow lookup and PDF template retrieval; rely on the existing logger.info call instead. Specifically, delete the print("flow_route: ", flow_name), print("Flow found: ", flow) and print("PDF Template: ", pdf_template) lines that appear around the flow_name/flow and PDFTemplates.objects.filter(...).first() calls so the code only uses the logger.info message that follows. Ensure no other side-effects remain after removing these print statements.chatbot/llm_models/llm_gateway.py (1)
63-63: ⚡ Quick winConsider making the timeout configurable.
The 120-second timeout is hard-coded in both
call_llm_gatewayandcall_llm_gateway_stream. Different LLM models, request sizes, or deployment environments may require different timeout values. Consider reading from an environment variable or accepting it as a parameter.♻️ Proposed refactor to make timeout configurable
+_DEFAULT_TIMEOUT = int(os.getenv('LLM_GATEWAY_TIMEOUT', '120')) + def call_llm_gateway( messages: list, provider: str, model: str, params: dict = None, tools: list = None, - tool_choice=None, + tool_choice=None, timeout: int = None, ) -> dict | None: """ POST to /v1/chat/ on the LLM gateway service. """ + timeout = timeout or _DEFAULT_TIMEOUT # ... existing code ... try: - response = requests.post(url, headers=headers, json=payload, timeout=120) + response = requests.post(url, headers=headers, json=payload, timeout=timeout)Apply the same pattern to
call_llm_gateway_stream.🤖 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 `@chatbot/llm_models/llm_gateway.py` at line 63, The hard-coded 120s timeout in call_llm_gateway and call_llm_gateway_stream should be made configurable: add a timeout parameter to both functions with a default value (e.g., 120) and/or read a fallback from an environment variable (e.g., LLM_GATEWAY_TIMEOUT parsed as int), then replace the literal 120 in the requests.post timeout argument with that variable; ensure you validate/convert the env var to an int and preserve current behavior when not set.chatbot/services/response_handlers/common_handler.py (1)
736-740: 💤 Low valueConsider making the response transformation more explicit.
The current code mutates the
responsedict in place usingpop(),clear(), andupdate(). While functionally correct, this mutation pattern can be surprising to readers. Consider extracting the normalized data first, then reassigning.♻️ Proposed refactor for clarity
if company_bot.bot_type == CompanyBotTypeChoices.SIMPLE: if response and isinstance(response, dict): - extracted_data = response.pop("parameters", response.pop("input", None)) - if extracted_data and isinstance(extracted_data, dict): - response.clear() - response.update(extracted_data) + extracted_data = response.get("parameters") or response.get("input") + if extracted_data and isinstance(extracted_data, dict): + response = extracted_dataNote: This requires updating the function signature to return the potentially new
responsevalue, or accepting that the original caller's reference remains unchanged.🤖 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 `@chatbot/services/response_handlers/common_handler.py` around lines 736 - 740, The current in-place mutation of the response dict (using response.pop(...), response.clear(), response.update(...)) is surprising; instead, read the normalized data into a new variable (e.g., normalized = response.get("parameters") or response.get("input")), check it is a dict, then either assign response = normalized (and update the function signature to return the possibly-new response) or explicitly replace the original mapping with a new dict via a single assignment to avoid pop/clear/update side effects; refer to the response variable and the "parameters"/"input" keys to locate and replace the mutation logic in common_handler.py.chatbot/services/response_handlers/base_response_handler.py (2)
394-394: ⚡ Quick winReplace debug print statements with logger calls.
Multiple debug print statements throughout this file bypass the logging infrastructure. Convert them to
logger.debug()calls to ensure they're captured by log aggregation tools and can be filtered by log level in production.♻️ Example conversion
-print(f'[tool_loop] iteration={iteration} result={str(result)[:200]}') +logger.debug(f'[tool_loop] iteration={iteration} result={str(result)[:200]}') -print("="*50) -print("Result: ", result) -print("="*50) +logger.debug("="*50) +logger.debug("Result: %s", result) +logger.debug("="*50)Apply similar changes to all print statements in this file.
Also applies to: 396-398, 526-526, 602-602, 636-636, 672-672, 756-758
🤖 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 `@chatbot/services/response_handlers/base_response_handler.py` at line 394, Replace all direct print(...) debug statements in base_response_handler.py with logger.debug(...) so logs flow through the app's logging infra; specifically update the print in the tool_loop function (the line printing iteration and result), and the other prints noted (around the blocks that currently print debug info at the locations you flagged) to call the module/class logger (e.g., self.logger.debug(...) if inside instance methods or logger.debug(...) for module-level code). Ensure a logger is available in the scope (use an existing logger variable or add: logger = logging.getLogger(__name__) or attach self.logger in the class constructor) and preserve the original message formatting (truncate result like str(result)[:200]) when converting to logger.debug calls.
363-363: ⚡ Quick winExtract max_iterations as a class constant.
The value
5for maximum tool iterations is hard-coded. Consider extracting it as a class constant (e.g.,MAX_TOOL_ITERATIONS = 5) in__init__to make it easier to adjust for different bot configurations or to document the rationale for this limit.🤖 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 `@chatbot/services/response_handlers/base_response_handler.py` at line 363, The hard-coded local variable max_iterations = 5 in BaseResponseHandler should be turned into a class-level constant so it’s configurable and documented; add a constant like MAX_TOOL_ITERATIONS = 5 on the BaseResponseHandler class (or assign self.MAX_TOOL_ITERATIONS in __init__ if instance-level is preferred), then replace usages of max_iterations with that constant (e.g., in the method referencing max_iterations) and update any docstring or comment to explain the rationale.chatbot/services/vector/vector_service.py (2)
11-11: ⚡ Quick winReplace print statements with logger calls.
Debug print statements at lines 11 and 38 will bypass the logging infrastructure and won't be captured by log aggregation tools. Use the existing
loggerinstance instead for consistent observability.♻️ Proposed refactor
- print("result: ", result) + logger.debug(f'[vector_service] query result: {result}')- print(f'[vector_service] query="{query}" → 0 chunks found') + # (This line can be removed since logger.info already logs the same information on line 37)Also applies to: 38-38
🤖 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 `@chatbot/services/vector/vector_service.py` at line 11, Replace the direct print calls in chatbot/services/vector/vector_service.py (the occurrences printing "result: " and any other debug prints) with the module's logger instance (e.g., logger.debug or logger.info as appropriate) so logs flow through the existing logging infrastructure; locate the prints (e.g., the print("result: ", result) and the second debug print) and change them to logger.debug(f"result: {result}") or logger.info(...) depending on desired level, preserving the message content and variable interpolation.
18-18: 💤 Low valueConsider extracting the minimum text length threshold as a constant.
The value
20for minimum text length is hard-coded. Extracting it as a module-level constant (e.g.,_MIN_CHUNK_TEXT_LENGTH = 20) would make it easier to tune for different knowledge bases or use cases.🤖 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 `@chatbot/services/vector/vector_service.py` at line 18, The hard-coded minimum chunk length (20) in the condition "if text and len(text) > 20 and score >= filter_score:" should be extracted to a module-level constant (e.g., _MIN_CHUNK_TEXT_LENGTH = 20) and the condition updated to use that constant (len(text) > _MIN_CHUNK_TEXT_LENGTH); update any related docstring/comments and tests that reference the magic number so the threshold is easy to tune for different knowledge bases and use cases.chatbot/services/core/orchestrator.py (1)
76-76: ⚡ Quick winRemove unused import.
The
jsonmodule is imported but not used in this code block. The sources block is constructed using string formatting without JSON serialization.♻️ Proposed fix
finalized_sources = (chat_session.other_params or {}).get('finalized_sources') if finalized_sources: - import json as _json sources_block = '\n'.join( f'- {s.get("title", "")} — {s.get("url", "")}' for s in finalized_sources )🤖 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 `@chatbot/services/core/orchestrator.py` at line 76, Remove the unused import "import json as _json" from the module; the symbol _json is never referenced when building the sources string, so delete that import line (or consolidate imports if needed) to eliminate the unused dependency and linter warning in orchestrator.py.
🤖 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 `@chatbot/celery_tasks/common_chat_tasks.py`:
- Around line 14-20: _safe_json_dumps currently re-serializes string inputs
which double-encodes pre-serialized payloads like `chunks`; update
_safe_json_dumps so it returns string inputs unchanged (e.g., if
isinstance(value, str): return value) before attempting json.dumps, keep the
None check and the try/except for non-string values to preserve existing
error-safe behavior.
- Around line 44-46: The append branch for append_to_last can raise TypeError
when message is None; in the function/block that updates last_chat.message
(referencing variables append_to_last, last_chat, and message) guard against
None by normalizing message to an empty string before concatenation (e.g., use
(message or '') or skip the append when message is None) so the expression
(last_chat.message or '') + '\n\n' + message never tries to add None to a str;
update that assignment accordingly.
In `@chatbot/llm_models/llm_gateway.py`:
- Around line 8-10: The module currently sets _API_KEY and _TENANT_ID to empty
strings by default which can silently produce auth failures; add validation that
checks _API_KEY and _TENANT_ID at module load (or at first use in the function
that sends requests) and raise a clear exception (e.g., ValueError) if either is
missing, so code using _API_KEY/_TENANT_ID (refer to those symbols) fails fast
with a descriptive message instead of sending empty Authorization or X-Tenant-Id
headers.
In `@chatbot/utils/media_preview/media_creation.py`:
- Around line 407-413: The NameError occurs because title_text is referenced but
never set in the non-MIP branch; fix by deriving title_text from the passed
arguments before calling doc.add_heading — e.g. set title_text =
arguments.get('title') or fallback to arguments.get('knowledge_title') or a
sensible default like "Document" — then use that title_text when calling
doc.add_heading and keep the existing loop that writes knowledge_content; update
the block around doc.add_heading, content = arguments.get('knowledge_content',
''), and the subsequent paragraph loop to use the new title_text variable.
In `@sample.env`:
- Around line 1-54: The environment variable template is missing several
variables used in settings.py and celery_config.py such as REDIS_HOST,
REDIS_PORT, REDIS_USE_SSL, REDIS_DB, PG_SSL_MODE, PG_SSL_ROOT_CERT,
CSRF_TRUSTED_ORIGINS, SENTRY_DSN, and the GCP/Azure storage keys
GCS_BUCKET_NAME, GCP_PROJECT_ID, GCP_CREDENTIALS_PATH, AZURE_ACCOUNT_NAME,
AZURE_ACCOUNT_KEY, and AZURE_CONTAINER_NAME. To fix this, add these missing
variables with appropriate placeholders or default values in the template to
clearly document them and prevent deployment issues.
---
Outside diff comments:
In `@chatbot/admin/story_admin.py`:
- Around line 1-184: The file chat_admin/story_admin.py is fully commented out
but is imported by chatbot/admin/__init__.py causing admin registrations and
routes (StoryAdmin, StoryTranslationAdmin, export_stories_view, save_model PDF
hooks, PostProcessingView route) to be missing; either delete story_admin.py if
removal is intentional, or restore minimal admin functionality by re-enabling
the StoryAdmin and StoryTranslationAdmin registrations (or create a minimal stub
that defines StoryAdmin and StoryTranslationAdmin as no-op admin.ModelAdmin
subclasses or safely guards the import) and add a clear TODO comment with an
expiry date; ensure the symbols StoryAdmin, StoryTranslationAdmin,
export_stories_view (or PostProcessingView route), and save_model logic are
present or stubbed so chatbot/admin/__init__.py import no longer breaks.
In `@chatbot/models/company_models.py`:
- Around line 73-94: The model defines provider and provider_keys twice causing
the first declarations to be shadowed; remove the duplicate early definitions
and keep the intended variants that use default=LLMProvider.BEDROCK and include
the help_text. Specifically, delete the initial provider = models.CharField(...)
and provider_keys = models.TextField(...) occurrences and ensure only the
provider and provider_keys fields with default=LLMProvider.BEDROCK and the
updated help_text remain in the model (referencing the provider and
provider_keys attributes and LLMProvider choices).
---
Nitpick comments:
In `@chatbot/admin/theme_admin.py`:
- Around line 1-40: The file contains a fully commented-out Django admin class
(ThemeAdmin) and related imports (Theme, ThemeType, get_form, changeform_view) —
remove the file entirely if the Theme admin is permanently disabled; if it's
temporary, restore a minimal, documented placeholder (keep a short module-level
comment explaining why it's disabled and a TODO with an expected re-enable date
or issue ref) or re-enable by uncommenting and ensuring ThemeAdmin, get_form and
changeform_view are correct; commit the deletion or the documented placeholder
so the repo no longer contains a whole commented-out implementation.
In `@chatbot/llm_models/llm_gateway.py`:
- Line 63: The hard-coded 120s timeout in call_llm_gateway and
call_llm_gateway_stream should be made configurable: add a timeout parameter to
both functions with a default value (e.g., 120) and/or read a fallback from an
environment variable (e.g., LLM_GATEWAY_TIMEOUT parsed as int), then replace the
literal 120 in the requests.post timeout argument with that variable; ensure you
validate/convert the env var to an int and preserve current behavior when not
set.
In `@chatbot/services/core/orchestrator.py`:
- Line 76: Remove the unused import "import json as _json" from the module; the
symbol _json is never referenced when building the sources string, so delete
that import line (or consolidate imports if needed) to eliminate the unused
dependency and linter warning in orchestrator.py.
In `@chatbot/services/response_handlers/base_response_handler.py`:
- Line 394: Replace all direct print(...) debug statements in
base_response_handler.py with logger.debug(...) so logs flow through the app's
logging infra; specifically update the print in the tool_loop function (the line
printing iteration and result), and the other prints noted (around the blocks
that currently print debug info at the locations you flagged) to call the
module/class logger (e.g., self.logger.debug(...) if inside instance methods or
logger.debug(...) for module-level code). Ensure a logger is available in the
scope (use an existing logger variable or add: logger =
logging.getLogger(__name__) or attach self.logger in the class constructor) and
preserve the original message formatting (truncate result like
str(result)[:200]) when converting to logger.debug calls.
- Line 363: The hard-coded local variable max_iterations = 5 in
BaseResponseHandler should be turned into a class-level constant so it’s
configurable and documented; add a constant like MAX_TOOL_ITERATIONS = 5 on the
BaseResponseHandler class (or assign self.MAX_TOOL_ITERATIONS in __init__ if
instance-level is preferred), then replace usages of max_iterations with that
constant (e.g., in the method referencing max_iterations) and update any
docstring or comment to explain the rationale.
In `@chatbot/services/response_handlers/common_handler.py`:
- Around line 736-740: The current in-place mutation of the response dict (using
response.pop(...), response.clear(), response.update(...)) is surprising;
instead, read the normalized data into a new variable (e.g., normalized =
response.get("parameters") or response.get("input")), check it is a dict, then
either assign response = normalized (and update the function signature to return
the possibly-new response) or explicitly replace the original mapping with a new
dict via a single assignment to avoid pop/clear/update side effects; refer to
the response variable and the "parameters"/"input" keys to locate and replace
the mutation logic in common_handler.py.
In `@chatbot/services/vector/vector_service.py`:
- Line 11: Replace the direct print calls in
chatbot/services/vector/vector_service.py (the occurrences printing "result: "
and any other debug prints) with the module's logger instance (e.g.,
logger.debug or logger.info as appropriate) so logs flow through the existing
logging infrastructure; locate the prints (e.g., the print("result: ", result)
and the second debug print) and change them to logger.debug(f"result: {result}")
or logger.info(...) depending on desired level, preserving the message content
and variable interpolation.
- Line 18: The hard-coded minimum chunk length (20) in the condition "if text
and len(text) > 20 and score >= filter_score:" should be extracted to a
module-level constant (e.g., _MIN_CHUNK_TEXT_LENGTH = 20) and the condition
updated to use that constant (len(text) > _MIN_CHUNK_TEXT_LENGTH); update any
related docstring/comments and tests that reference the magic number so the
threshold is easy to tune for different knowledge bases and use cases.
In `@chatbot/utils/media_preview/media_creation.py`:
- Around line 291-295: Remove the three debug print() calls around flow lookup
and PDF template retrieval; rely on the existing logger.info call instead.
Specifically, delete the print("flow_route: ", flow_name), print("Flow found: ",
flow) and print("PDF Template: ", pdf_template) lines that appear around the
flow_name/flow and PDFTemplates.objects.filter(...).first() calls so the code
only uses the logger.info message that follows. Ensure no other side-effects
remain after removing these print statements.
In `@shikshalokam_mohini/settings.py`:
- Line 54: SECRET_KEY is set from os.getenv('DJANGO_SECRET_KEY') which can be
None; add a fail-fast guard in settings so that if SECRET_KEY is falsy you raise
a clear exception at import time. Locate the SECRET_KEY assignment and follow-up
use of SIGNING_KEY (SIGNING_KEY = SECRET_KEY) and replace with logic that
validates SECRET_KEY (e.g., check SECRET_KEY after assignment) and raise a
RuntimeError or ImproperlyConfigured with a clear message instructing to set
DJANGO_SECRET_KEY when missing.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 78931d06-c947-4223-8575-2c971fe9e37a
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
chatbot/admin/story_admin.pychatbot/admin/theme_admin.pychatbot/celery_tasks/common_chat_tasks.pychatbot/llm_models/llm_gateway.pychatbot/migrations/0082_companybot_use_vector_service_and_more.pychatbot/migrations/0083_alter_companybot_llm_model_alter_companybot_provider_and_more.pychatbot/migrations/0084_companybot_enable_web_search_and_more.pychatbot/models/company_models.pychatbot/models/enums.pychatbot/routing.pychatbot/services/core/orchestrator.pychatbot/services/core/prompt_builder.pychatbot/services/response_handlers/base_response_handler.pychatbot/services/response_handlers/common_handler.pychatbot/services/vector/__init__.pychatbot/services/vector/vector_service.pychatbot/utils/chat_utils.pychatbot/utils/media_preview/media_creation.pyobservability/migrations/0007_alter_botruntestcasemap_metric_name_and_more.pyobservability/migrations/0008_alter_companybottcrun_llm_model_and_more.pypyproject.tomlsample.envshikshalokam_mohini/celery_config.pyshikshalokam_mohini/settings.py
💤 Files with no reviewable changes (1)
- chatbot/routing.py
This was referenced Jun 17, 2026
Merged
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
2 participants
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.
Summary by CodeRabbit
New Features
Enhancements
Chores