Add Grok API support and improve LLM integration#163
Merged
ovchynnikov merged 27 commits intoovchynnikov:mainfrom Mar 11, 2026
Merged
Add Grok API support and improve LLM integration#163ovchynnikov merged 27 commits intoovchynnikov:mainfrom
ovchynnikov merged 27 commits intoovchynnikov:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds multi-provider LLM support (Grok, Gemini) with dynamic selection, per-user rate and daily limits, optional conversation context, SQLite-backed persistence (BotStorage), API retry/safety logic, background cleanup of stale users, and Docker/docker-compose changes to enable persistent storage. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Bot as Bot (src/main.py)
participant Storage as BotStorage (src/db_storage.py)
participant LLM as LLM API (Grok / Gemini)
User->>Bot: Send message
activate Bot
Bot->>Storage: load_user_data(user_id)
Storage-->>Bot: return context, limits, last_seen
alt rate/daily limit exceeded
Bot-->>User: localized rate-limit message
else
alt USE_CONVERSATION_CONTEXT
Bot->>Bot: Build context-aware prompt
else
Bot->>Bot: Build prompt from message
end
Bot->>LLM: call_grok_api / call_gemini_api(prompt)
activate LLM
LLM-->>Bot: response or error/rate-limit
deactivate LLM
alt API error or blocked
Bot->>Bot: apply retry / safety fallback
else
Bot->>Bot: normalize (strip Markdown)
end
Bot->>Storage: save_user_data(updated context, limits, last_seen)
Storage-->>Bot: ack
Bot-->>User: Send response
end
deactivate Bot
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/main.py (5)
589-589: Minor: Redundantuser_idassignment.
user_idis already assigned on line 523. This reassignment is unnecessary.- user_id = update.effective_user.id if USE_CONVERSATION_CONTEXT:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` at line 589, Remove the redundant assignment to user_id (the line assigning update.effective_user.id) since user_id is already set earlier; delete the second assignment and ensure subsequent code in the function uses the originally declared user_id variable (refer to the earlier user_id declaration and any uses of update.effective_user.id in this function to confirm no other references need adjustment).
625-626: Movetracebackimport to file top.Importing
tracebackinside the exception handler works but is non-idiomatic. Standard practice is to import at the module level for clarity and slight performance benefit (avoids repeated import overhead).Add to imports at top of file:
import tracebackThen remove line 626.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` around lines 625 - 626, Move the traceback import out of the exception handler: add "import traceback" to the module-level imports at the top of src/main.py and remove the inline "import traceback" from the except Exception as e: handler so the exception block uses the top-level traceback import; locate the inline import in the except Exception as e: block to delete it and ensure any traceback calls there continue to work with the new module-level import.
504-520: UnknownLLM_PROVIDERvalues fall through to Gemini path silently.If
LLM_PROVIDERis set to an unsupported value (e.g., "openai", "claude"), the validation passes (neithergroknorgeminimatch), and line 607'selsebranch routes to Gemini. Consider validating the provider early.💡 Suggested validation
+ # Validate provider + if LLM_PROVIDER not in ("grok", "gemini"): + error("Unknown LLM_PROVIDER: %s", LLM_PROVIDER) + bot_response = ( + "Вибачте, невідомий AI провайдер." + if language == "uk" + else "Sorry, unknown AI provider configured." + ) + await update.message.reply_text(bot_response) + return + # Check if API is configured if LLM_PROVIDER == "grok" and not GROK_API_KEY:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` around lines 504 - 520, The provider validation currently only checks for "grok" and "gemini" so unsupported LLM_PROVIDER values fall through; add an explicit allowed-provider check (e.g., ALLOWED_PROVIDERS = {"grok","gemini"}) and validate LLM_PROVIDER against it before the existing checks, returning an error message via update.message.reply_text if the provider is not supported, preserving the language conditional; update the block that references LLM_PROVIDER, GROK_API_KEY, GEMINI_API_KEY and reply handling to short-circuit on invalid provider and avoid silently routing to the Gemini branch.
765-770: Bareexceptsuppresses all exceptions includingKeyboardInterrupt.Line 765 uses a bare
except:which catches everything includingSystemExitandKeyboardInterrupt. While the# --- IGNORE ---comment suggests this is intentional, consider catchingExceptioninstead for cleaner semantics.- except: # --- IGNORE --- # pylint: disable=bare-except + except Exception: # pylint: disable=broad-exception-caught🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` around lines 765 - 770, The bare except block currently catches everything (including KeyboardInterrupt/SystemExit); change it to catch only exceptions by replacing "except:" with "except Exception:" (and optionally log the exception) in the handler that returns the localized apology message (the block referencing language and returning the Ukrainian/English strings) so system-level interrupts are not suppressed.
650-664: Add defensive check forgrok_clientbeing None.While the caller checks
GROK_API_KEY, if client initialization fails silently or under edge cases,grok_client.chat.completions.create()would raiseAttributeError: 'NoneType' object has no attribute 'chat'. Consider adding a guard.💡 Suggested guard
async def call_grok_api(safe_prompt: str, update) -> str: """Call Grok API and return response.""" + if grok_client is None: + return ( + "Вибачте, Grok API клієнт не ініціалізовано." + if language == "uk" + else "Sorry, Grok API client is not initialized." + ) try: max_retries = 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` around lines 650 - 664, call_grok_api currently assumes grok_client is initialized and will raise AttributeError if grok_client is None; add a defensive guard at the start of call_grok_api that checks if grok_client is None and handles it (e.g., log an error using the existing logger/update, raise a clear RuntimeError, or return a sensible fallback string) before attempting grok_client.chat.completions.create; reference the grok_client variable and GROK_MODEL in the check to make the intent clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.py`:
- Around line 549-551: The current code appends current_time to
llm_rate_limit[user_id] and increments llm_daily_limit[user_id] before the LLM
API call, so failed calls consume quota; move the increment of llm_daily_limit
and the final append (or make the append provisional) so they only occur after a
successful API response from the LLM, or wrap the API call in
try/except/finally: tentatively append current_time to llm_rate_limit[user_id],
call the API, on success perform llm_daily_limit[user_id] += 1 (and keep the
timestamp), on exception remove the tentative timestamp from
llm_rate_limit[user_id] (and re-raise) so failures do not decrement user quota;
reference symbols: llm_rate_limit, llm_daily_limit, current_time, user_id, and
the LLM API call block.
- Around line 59-68: The llm_daily_limit counter never resets so users can be
permanently blocked; update the logic in respond_with_llm_message to track daily
usage keyed by date instead of a raw int: change llm_daily_limit to map user_id
-> {"count": int, "date": "YYYY-MM-DD"} (or store a tuple) and on each request
compare the stored date to today's date, resetting count to 0 when the date
differs before enforcing LLM_RPD_LIMIT, then increment the count after a
successful allowance; reference llm_daily_limit, respond_with_llm_message, and
LLM_RPD_LIMIT when making this change and mirror the cleanup semantics used for
llm_rate_limit.
---
Nitpick comments:
In `@src/main.py`:
- Line 589: Remove the redundant assignment to user_id (the line assigning
update.effective_user.id) since user_id is already set earlier; delete the
second assignment and ensure subsequent code in the function uses the originally
declared user_id variable (refer to the earlier user_id declaration and any uses
of update.effective_user.id in this function to confirm no other references need
adjustment).
- Around line 625-626: Move the traceback import out of the exception handler:
add "import traceback" to the module-level imports at the top of src/main.py and
remove the inline "import traceback" from the except Exception as e: handler so
the exception block uses the top-level traceback import; locate the inline
import in the except Exception as e: block to delete it and ensure any traceback
calls there continue to work with the new module-level import.
- Around line 504-520: The provider validation currently only checks for "grok"
and "gemini" so unsupported LLM_PROVIDER values fall through; add an explicit
allowed-provider check (e.g., ALLOWED_PROVIDERS = {"grok","gemini"}) and
validate LLM_PROVIDER against it before the existing checks, returning an error
message via update.message.reply_text if the provider is not supported,
preserving the language conditional; update the block that references
LLM_PROVIDER, GROK_API_KEY, GEMINI_API_KEY and reply handling to short-circuit
on invalid provider and avoid silently routing to the Gemini branch.
- Around line 765-770: The bare except block currently catches everything
(including KeyboardInterrupt/SystemExit); change it to catch only exceptions by
replacing "except:" with "except Exception:" (and optionally log the exception)
in the handler that returns the localized apology message (the block referencing
language and returning the Ukrainian/English strings) so system-level interrupts
are not suppressed.
- Around line 650-664: call_grok_api currently assumes grok_client is
initialized and will raise AttributeError if grok_client is None; add a
defensive guard at the start of call_grok_api that checks if grok_client is None
and handles it (e.g., log an error using the existing logger/update, raise a
clear RuntimeError, or return a sensible fallback string) before attempting
grok_client.chat.completions.create; reference the grok_client variable and
GROK_MODEL in the check to make the intent clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a07fc94e-a62f-423b-96ea-e611f5c4465a
📒 Files selected for processing (2)
src/main.pysrc/requirements.txt
- Move traceback import to module level to avoid inline imports - Add provider validation with ALLOWED_PROVIDERS set - Replace bare except with except Exception and add logging - Implement daily limit reset using date tracking - Fix quota consumption: only increment counters after successful API calls - Add tentative timestamp mechanism that rolls back on failures
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main.py (2)
609-609: Redundant variable assignment.
user_idis already assigned at line 538 and hasn't changed. This reassignment is unnecessary.♻️ Suggested fix
- # Build context from previous messages if enabled - user_id = update.effective_user.id + # Build context from previous messages if enabled🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` at line 609, Remove the redundant reassignment "user_id = update.effective_user.id" (the variable user_id was already set earlier) and use the originally assigned user_id variable; if the intent was to refresh the user ID, replace the duplicate assignment with a comment or an explicit check that update.effective_user differs before reassigning. Locate the duplicate by searching for the symbol user_id and the expression update.effective_user.id in src/main.py and delete or replace the second occurrence.
62-70: Consider periodic cleanup of stale user data for long-running deployments.The
conversation_context,llm_rate_limit, andllm_daily_limitdictionaries grow indefinitely with new users. Whilellm_rate_limitcleans old timestamps per user, the user entries themselves persist. For bots with many users over time, this could accumulate memory.Consider adding periodic cleanup for users who haven't interacted recently, or using a bounded cache (e.g., LRU) if memory becomes a concern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` around lines 62 - 70, The global dicts conversation_context, llm_rate_limit and llm_daily_limit can grow unbounded; add a periodic cleanup job (e.g., a function named cleanup_stale_users or start_cleanup_task) that runs on a timer and removes entries for users that haven't had activity for a configurable TTL (use a new per-user last_seen timestamp tracked when requests arrive), or replace these structures with a bounded cache/LRU implementation (e.g., wrap them with an LRU cache class or use cachetools TTLCache) to automatically evict old users; ensure the cleanup references the existing symbols conversation_context, llm_rate_limit, llm_daily_limit and updates/clears their entries safely (with the same locking/async model the app uses) and expose TTL and interval via environment variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.py`:
- Around line 634-646: The helper functions call_grok_api and call_gemini_api
currently swallow exceptions and return error strings, causing
respond_with_llm_message to treat failures as successes; change those helpers to
propagate errors (remove the outer try/except or re-raise caught exceptions) so
callers can detect failures, and update respond_with_llm_message so it only
increments llm_daily_limit[user_id]["count"], records the timestamp, and appends
to conversation_context[user_id] after a successful API call (i.e., when no
exception was raised and a valid bot_response was obtained); alternatively, if
you keep error-returning helpers, change them to return a clear success/failure
sentinel (e.g., (success, response)) and branch in respond_with_llm_message to
only perform the increment/store on success.
---
Nitpick comments:
In `@src/main.py`:
- Line 609: Remove the redundant reassignment "user_id =
update.effective_user.id" (the variable user_id was already set earlier) and use
the originally assigned user_id variable; if the intent was to refresh the user
ID, replace the duplicate assignment with a comment or an explicit check that
update.effective_user differs before reassigning. Locate the duplicate by
searching for the symbol user_id and the expression update.effective_user.id in
src/main.py and delete or replace the second occurrence.
- Around line 62-70: The global dicts conversation_context, llm_rate_limit and
llm_daily_limit can grow unbounded; add a periodic cleanup job (e.g., a function
named cleanup_stale_users or start_cleanup_task) that runs on a timer and
removes entries for users that haven't had activity for a configurable TTL (use
a new per-user last_seen timestamp tracked when requests arrive), or replace
these structures with a bounded cache/LRU implementation (e.g., wrap them with
an LRU cache class or use cachetools TTLCache) to automatically evict old users;
ensure the cleanup references the existing symbols conversation_context,
llm_rate_limit, llm_daily_limit and updates/clears their entries safely (with
the same locking/async model the app uses) and expose TTL and interval via
environment variables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 50eebe79-9a38-4608-a100-5b2e7d5e6b5e
📒 Files selected for processing (2)
PR_MESSAGE.mdsrc/main.py
…ction to Grok - Remove duplicate user_id assignment - Make call_grok_api and call_gemini_api raise exceptions instead of returning error strings - Add plain text system instruction to Grok API for consistent formatting - Ensure quota counters only increment after successful API responses - Add periodic cleanup task to prevent unbounded memory growth of user data
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/db_storage.py (1)
23-33: Add an index forlast_seento keep cleanup queries scalable.Line 82 performs range filtering on
last_seen; without an index this becomes a full table scan as data grows.⚡ Suggested schema improvement
cursor.execute(""" CREATE TABLE IF NOT EXISTS user_data ( user_id INTEGER PRIMARY KEY, conversation_context TEXT, rate_limit_timestamps TEXT, daily_count INTEGER DEFAULT 0, daily_date TEXT, last_seen REAL ) """) + cursor.execute( + "CREATE INDEX IF NOT EXISTS idx_user_data_last_seen ON user_data(last_seen)" + ) self.conn.commit()Also applies to: 82-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db_storage.py` around lines 23 - 33, Add a non-unique index on user_data.last_seen in the DB schema setup to avoid full table scans during cleanup queries that filter by last_seen; modify the table-creation block (user_data) to run a CREATE INDEX IF NOT EXISTS idx_user_data_last_seen ON user_data(last_seen) and commit the change so the cleanup/range-filter operations (the code that filters by last_seen) become scalable.src/main.py (1)
902-905: Store the cleanup task reference and cancel it on shutdown.Line 903 creates an infinite background task without retaining a handle. To ensure graceful shutdown, store the task reference and cancel it via a
post_shutdownhandler:cleanup_task = None async def post_init(app): global cleanup_task cleanup_task = asyncio.create_task(cleanup_stale_users()) async def post_shutdown(app): global cleanup_task if cleanup_task: cleanup_task.cancel() application.post_init = post_init application.post_shutdown = post_shutdownWhile
Application.run_polling()may implicitly clean up tasks, explicit lifecycle management prevents potential issues with unfinished coroutines during shutdown.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` around lines 902 - 905, The post_init currently launches cleanup_stale_users() via asyncio.create_task without keeping the Task; create a module-level variable (e.g., cleanup_task) and assign cleanup_task = asyncio.create_task(cleanup_stale_users()) inside post_init (use global if needed), then implement an async post_shutdown(app) that checks if cleanup_task is not None and calls cleanup_task.cancel() (optionally await it or handle CancelledError), and finally set application.post_shutdown = post_shutdown while keeping application.post_init = post_init so the background task is cancelled on shutdown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 31-43: The README's fenced code blocks for the Docker commands
(the three docker run examples and the instagram cookies example) are missing
language identifiers and trigger MD040; update each triple-backtick fence that
wraps those commands to use a bash language tag (e.g., replace ``` with ```bash)
so all four code blocks are annotated (the docker run without volume, the docker
run using Docker Hub image, the docker run with persistent volume, and the
instagram cookies docker run).
In `@src/main.py`:
- Around line 550-551: When merging persisted last-seen data back into the
in-memory map, avoid clobbering a fresher value with an older DB value: instead
of unconditionally assigning the DB value (the variable referenced as
persisted_last_seen or db_last_seen) to user_last_seen[user_id], only set it if
the key is missing or the DB timestamp is newer than the existing
user_last_seen[user_id]; apply the same check everywhere you merge DB values
(the code around user_last_seen, user_id, current_time and the DB load/merge
sites noted in the review) so current_time updates are preserved and stale DB
timestamps are ignored.
- Around line 650-653: The prompt text is hardcoded to Ukrainian in the
construction of safe_prompt (and context_str usage), breaking localization when
LANGUAGE=en; modify the logic that builds safe_prompt to choose the instruction
language dynamically based on the current language/config (e.g., the existing
LANGUAGE or locale variable) instead of the fixed Ukrainian string — use a
conditional or mapping to insert either the Ukrainian instruction ("Відповідай
українською...") when language is 'uk' or the equivalent English instruction
("Answer in English...") when language is 'en' (apply the same change both in
the branch that includes context_str and the else branch so safe_prompt respects
the selected language consistently).
- Around line 553-557: The code is calling synchronous DB methods
(db_storage.load_user_data, save_user_data, get_stale_users, delete_user_data)
directly inside async handlers (e.g., in llm_daily_limit load path and in async
functions respond_with_llm_message and cleanup_stale_users) which blocks the
event loop; fix by offloading those synchronous calls to a thread executor
(e.g., using asyncio.get_running_loop().run_in_executor or asyncio.to_thread)
and await the result, replacing direct calls like
db_storage.load_user_data(user_id) with an awaited executor call, and do the
same for save_user_data/get_stale_users/delete_user_data throughout the
referenced functions to ensure non-blocking behavior.
- Around line 767-772: The safety_settings dict currently sets BLOCK_NONE for
all categories; update safety_settings to use a stricter default (e.g.,
genai.types.HarmBlockThreshold.BLOCK_MEDIUM_AND_ABOVE or at minimum
genai.types.HarmBlockThreshold.BLOCK_ONLY_HIGH) for sensitive categories such as
HARM_CATEGORY_HARASSMENT, HARM_CATEGORY_HATE_SPEECH,
HARM_CATEGORY_SEXUALLY_EXPLICIT and HARM_CATEGORY_DANGEROUS_CONTENT; keep the
dict name safety_settings and the genai.types.HarmCategory/HarmBlockThreshold
symbols so the rest of the code continues to reference the same keys and values.
---
Nitpick comments:
In `@src/db_storage.py`:
- Around line 23-33: Add a non-unique index on user_data.last_seen in the DB
schema setup to avoid full table scans during cleanup queries that filter by
last_seen; modify the table-creation block (user_data) to run a CREATE INDEX IF
NOT EXISTS idx_user_data_last_seen ON user_data(last_seen) and commit the change
so the cleanup/range-filter operations (the code that filters by last_seen)
become scalable.
In `@src/main.py`:
- Around line 902-905: The post_init currently launches cleanup_stale_users()
via asyncio.create_task without keeping the Task; create a module-level variable
(e.g., cleanup_task) and assign cleanup_task =
asyncio.create_task(cleanup_stale_users()) inside post_init (use global if
needed), then implement an async post_shutdown(app) that checks if cleanup_task
is not None and calls cleanup_task.cancel() (optionally await it or handle
CancelledError), and finally set application.post_shutdown = post_shutdown while
keeping application.post_init = post_init so the background task is cancelled on
shutdown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f1bb5eeb-c58a-46ec-9cd3-a8bc75451d06
📒 Files selected for processing (7)
.gitignoreDockerfileREADME.mddocker-compose.ymlsrc/db_storage.pysrc/main.pysrc/requirements.txt
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- src/requirements.txt
Comment on lines
+31
to
43
| ``` | ||
| docker run -d --name downloader-bot --restart always --env-file .env -v bot-data:/bot/data downloader-bot:latest | ||
| ``` | ||
| or use a built image from **Docker hub** | ||
| ``` | ||
| docker run -d --name downloader-bot --restart always --env-file .env ovchynnikov/load-bot-linux:latest | ||
| ``` | ||
| With persistent data: | ||
| ``` | ||
| docker run -d --name downloader-bot --restart always --env-file .env -v bot-data:/bot/data ovchynnikov/load-bot-linux:latest | ||
| ``` | ||
| or if you use instagram cookies | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to new fenced code blocks.
Lines 31, 35, 39, and 43 trigger MD040 (fenced-code-language). Please annotate these command fences (e.g., bash) to keep docs lint-clean.
📝 Suggested doc fix
-```
+```bash
docker run -d --name downloader-bot --restart always --env-file .env -v bot-data:/bot/data downloader-bot:latest- +bash
docker run -d --name downloader-bot --restart always --env-file .env ovchynnikov/load-bot-linux:latest
-```
+```bash
docker run -d --name downloader-bot --restart always --env-file .env -v bot-data:/bot/data ovchynnikov/load-bot-linux:latest
- +bash
docker run -d --name downloader-bot --restart always --env-file .env -v bot-data:/bot/data -v /absolute/path/to/instagram_cookies.txt:/bot/instagram_cookies.txt ovchynnikov/load-bot-linux:latest
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 31 - 43, The README's fenced code blocks for the
Docker commands (the three docker run examples and the instagram cookies
example) are missing language identifiers and trigger MD040; update each
triple-backtick fence that wraps those commands to use a bash language tag
(e.g., replace ``` with ```bash) so all four code blocks are annotated (the
docker run without volume, the docker run using Docker Hub image, the docker run
with persistent volume, and the instagram cookies docker run).
Comment on lines
+767
to
+772
| safety_settings = { | ||
| genai.types.HarmCategory.HARM_CATEGORY_HARASSMENT: genai.types.HarmBlockThreshold.BLOCK_NONE, | ||
| genai.types.HarmCategory.HARM_CATEGORY_HATE_SPEECH: genai.types.HarmBlockThreshold.BLOCK_NONE, | ||
| genai.types.HarmCategory.HARM_CATEGORY_SEXUALLY_EXPLICIT: genai.types.HarmBlockThreshold.BLOCK_NONE, | ||
| genai.types.HarmCategory.HARM_CATEGORY_DANGEROUS_CONTENT: genai.types.HarmBlockThreshold.BLOCK_NONE, | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if src/main.py exists and read the relevant section
head -n 775 src/main.py | tail -n 20Repository: ovchynnikov/load-bot-linux
Length of output: 1263
🌐 Web query:
Google Generative AI HarmBlockThreshold BLOCK_NONE recommended settings public user-generated content chatbot safety
💡 Result:
For a public, user-generated-content (UGC) chatbot, don’t use BLOCK_NONE as your default.
- Why not
BLOCK_NONE:- In Vertex AI,
BLOCK_NONEremoves automated response blocking and shifts responsibility to you to enforce your own content guidelines using the returned safety scores; it’s also restricted (not available to all users) in GA model versions. [2] - In the Gemini API, using less restrictive safety settings can be subject to review under the Terms of Service. [1]
- For Gemini 2.5/3 models, extra adjustable filters are “off by default” because the model’s default behavior covers most use cases; you should only loosen/tune filters when you have a concrete need and have tested the impact. [1]
- In Vertex AI,
Recommended baseline for public UGC chatbots
Use blocking thresholds, not BLOCK_NONE, for the adjustable harm categories (harassment, hate speech, sexually explicit, dangerous):
- Safer default:
BLOCK_MEDIUM_AND_ABOVE(block medium + high) - Less restrictive (still moderated):
BLOCK_ONLY_HIGH(block only high)
These thresholds are explicitly supported in both Gemini API safety settings and Vertex AI safety settings. [1] [2]
When BLOCK_NONE can make sense
Only consider BLOCK_NONE if you intentionally want no automated blocking and you have a strong, tested moderation stack (pre-filter + post-filter + enforcement + logging/appeals), and (on Vertex) you have access to that restricted setting. [2]
Sources: Gemini API safety settings docs [1]; Vertex AI “Safety and content filters” docs [2].
Gemini safety is fully disabled for all harm categories.
Lines 768-772 set BLOCK_NONE across all content categories. Google's official documentation recommends using BLOCK_MEDIUM_AND_ABOVE (or BLOCK_ONLY_HIGH for less restrictive) for public, user-generated-content chatbots. Disabling automated safety checks entirely is subject to Terms of Service review and introduces significant compliance and moderation risk, particularly without a comprehensive pre- and post-filtering moderation stack in place.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main.py` around lines 767 - 772, The safety_settings dict currently sets
BLOCK_NONE for all categories; update safety_settings to use a stricter default
(e.g., genai.types.HarmBlockThreshold.BLOCK_MEDIUM_AND_ABOVE or at minimum
genai.types.HarmBlockThreshold.BLOCK_ONLY_HIGH) for sensitive categories such as
HARM_CATEGORY_HARASSMENT, HARM_CATEGORY_HATE_SPEECH,
HARM_CATEGORY_SEXUALLY_EXPLICIT and HARM_CATEGORY_DANGEROUS_CONTENT; keep the
dict name safety_settings and the genai.types.HarmCategory/HarmBlockThreshold
symbols so the rest of the code continues to reference the same keys and values.
- Add DB index on last_seen for efficient cleanup - Make all DB calls async with asyncio.to_thread - Fix stale timestamp handling from DB - Add dynamic localization for LLM prompts (uk/en) - Add proper cleanup task cancellation on shutdown
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main.py (1)
1-10:⚠️ Potential issue | 🟡 MinorBlack formatting check failed.
The CI pipeline reports that this file needs reformatting. Run
black --skip-string-normalization --line-length 120 src/to fix formatting issues before merging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` around lines 1 - 10, The file-level formatting (top-level docstring and imports in the module containing the triple-quoted string and imports like os, random, json, asyncio, re, time, traceback, datetime) fails Black checks; run the formatter with the exact CI command `black --skip-string-normalization --line-length 120 src/` (or run it against this module) and commit the resulting changes so the docstring and import block conform to project style.
🧹 Nitpick comments (3)
src/db_storage.py (3)
77-84: Moveimport timeto the top of the file.The
timeimport inside the method works but is non-standard. For consistency and clarity, place imports at module level.♻️ Proposed fix
Add to imports at top of file:
import sqlite3 import json import os +import time from logger import debugThen remove from method:
def get_stale_users(self, ttl_seconds): """Get list of user IDs that haven't been seen within TTL.""" - import time - current_time = time.time()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db_storage.py` around lines 77 - 84, Move the local import out of the get_stale_users method: add "import time" to the module-level imports at the top of the file and remove the "import time" statement inside the get_stale_users(self, ttl_seconds) method; keep the function body otherwise unchanged (retain current_time = time.time(), cursor usage, and return logic) to maintain behavior.
36-49: Positional row indexing is fragile if schema changes.Using
row[1],row[2], etc. depends on column order. If the schema is altered (columns added/reordered), this silently breaks. Consider using explicit column names in SELECT orsqlite3.Rowfactory.♻️ Proposed fix using explicit columns and Row factory
def load_user_data(self, user_id): """Load user data from database.""" cursor = self.conn.cursor() - cursor.execute("SELECT * FROM user_data WHERE user_id = ?", (user_id,)) + cursor.execute( + """SELECT user_id, conversation_context, rate_limit_timestamps, + daily_count, daily_date, last_seen + FROM user_data WHERE user_id = ?""", + (user_id,), + ) row = cursor.fetchone() if row: + # Explicit unpacking for clarity + _, ctx, timestamps, daily_count, daily_date, last_seen = row return { - "conversation_context": json.loads(row[1]) if row[1] else [], - "rate_limit_timestamps": json.loads(row[2]) if row[2] else [], - "daily_count": row[3], - "daily_date": row[4], - "last_seen": row[5], + "conversation_context": json.loads(ctx) if ctx else [], + "rate_limit_timestamps": json.loads(timestamps) if timestamps else [], + "daily_count": daily_count, + "daily_date": daily_date, + "last_seen": last_seen, } return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db_storage.py` around lines 36 - 49, The load_user_data method uses positional indexing on the fetched row (row[1], row[2], etc.), which is fragile; change it to fetch columns by name instead—either set the connection row factory to sqlite3.Row (self.conn.row_factory = sqlite3.Row) and then access row['conversation_context'], row['rate_limit_timestamps'], etc., or modify the SQL to select explicit column names and map them to keys; update load_user_data to use these named accesses (and keep the same JSON parsing and defaults) to avoid breakage if the schema changes.
12-18: Shared SQLite connection may cause "database is locked" errors under concurrent load.The single
self.connconnection withcheck_same_thread=Falseis shared across allasyncio.to_threadcalls. Under concurrent requests, multiple threads may attempt simultaneous writes, causing SQLite locking issues. Consider using a connection pool, creating connections per-operation, or adding a threading lock around DB operations.♻️ Proposed fix using connection-per-operation pattern
def __init__(self, db_path="data/bot.db"): """Initialize database connection and create tables.""" os.makedirs(os.path.dirname(db_path), exist_ok=True) self.db_path = db_path - self.conn = sqlite3.connect(db_path, check_same_thread=False) - self._create_tables() + # Create tables using a temporary connection + with sqlite3.connect(db_path) as conn: + self._create_tables(conn) debug("Database initialized at %s", db_path) + +def _get_connection(self): + """Get a new connection for thread-safe operations.""" + return sqlite3.connect(self.db_path)Then update each method to use
with self._get_connection() as conn:instead ofself.conn.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db_storage.py` around lines 12 - 18, The class currently creates and shares a single SQLite connection (self.conn) in __init__, which causes "database is locked" under concurrent asyncio.to_thread calls; change to a connection-per-operation pattern by removing reliance on self.conn, add a helper method _get_connection(self) that returns sqlite3.connect(self.db_path, check_same_thread=False) (or use sqlite3.connect(..., timeout=...)) and update all DB methods (including _create_tables, any read/write methods) to use a context manager like with self._get_connection() as conn: instead of using self.conn; this ensures each operation gets its own connection (or you can alternatively add a threading.Lock around writes if preferred).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/db_storage.py`:
- Around line 86-88: The post_shutdown handler in main.py currently only cancels
the cleanup task and never closes the DB connection; update that handler to call
db_storage.close() (the close() method on the db_storage instance) so the
SQLite/DB connection is released on shutdown, ensuring you reference the same
db_storage object used elsewhere (e.g., the global or injected instance) and
wrap the call in a try/except to log any errors without preventing shutdown.
In `@src/main.py`:
- Around line 931-942: post_shutdown currently only cancels cleanup_task and
never closes the SQLite DB; update post_shutdown to also close the application's
DB connection: reference the same global DB handle (e.g., the module-level
sqlite connection variable used elsewhere) inside post_shutdown after
cancelling/awaiting cleanup_task, check it's not None, call its close() method
(handling any exceptions like OperationalError) and set the global to None so
the connection is fully released before shutdown; keep the existing
asyncio.CancelledError handling and ensure application.post_shutdown remains
assigned to post_shutdown.
- Around line 843-849: The fallback uses a hardcoded Ukrainian instruction when
calling model.generate_content; change it to build the instruction using the
existing language setting (instead of the literal "Відповідь українською
мовою...") so the fallback respects the user's language preference—modify the
call to model.generate_content (the block that produces simple_response and
checks simple_response.text) to interpolate/format the language variable with
the same intent ("respond in <language>: give general information about:
<prompt>") and keep passing safety_settings and prompt as before.
- Around line 801-835: The retry loop around model.generate_content can exit
without setting response if all rate-limit retries are exhausted, leading to an
UnboundLocalError; fix this by ensuring a clear failure path after the for
attempt in range(max_retries) loop: initialize response = None before the loop
(or check locals() after the loop), and if response is still None after retries,
send a final user-facing message via update.message.reply_text (same language
logic as the retry message) and then raise a specific exception (or return/exit)
to stop further processing instead of letting execution continue to where
response is used; update the code around model.generate_content, the except
block handling rate-limit, and the post-loop logic to implement this
deterministic failure behavior.
---
Outside diff comments:
In `@src/main.py`:
- Around line 1-10: The file-level formatting (top-level docstring and imports
in the module containing the triple-quoted string and imports like os, random,
json, asyncio, re, time, traceback, datetime) fails Black checks; run the
formatter with the exact CI command `black --skip-string-normalization
--line-length 120 src/` (or run it against this module) and commit the resulting
changes so the docstring and import block conform to project style.
---
Nitpick comments:
In `@src/db_storage.py`:
- Around line 77-84: Move the local import out of the get_stale_users method:
add "import time" to the module-level imports at the top of the file and remove
the "import time" statement inside the get_stale_users(self, ttl_seconds)
method; keep the function body otherwise unchanged (retain current_time =
time.time(), cursor usage, and return logic) to maintain behavior.
- Around line 36-49: The load_user_data method uses positional indexing on the
fetched row (row[1], row[2], etc.), which is fragile; change it to fetch columns
by name instead—either set the connection row factory to sqlite3.Row
(self.conn.row_factory = sqlite3.Row) and then access
row['conversation_context'], row['rate_limit_timestamps'], etc., or modify the
SQL to select explicit column names and map them to keys; update load_user_data
to use these named accesses (and keep the same JSON parsing and defaults) to
avoid breakage if the schema changes.
- Around line 12-18: The class currently creates and shares a single SQLite
connection (self.conn) in __init__, which causes "database is locked" under
concurrent asyncio.to_thread calls; change to a connection-per-operation pattern
by removing reliance on self.conn, add a helper method _get_connection(self)
that returns sqlite3.connect(self.db_path, check_same_thread=False) (or use
sqlite3.connect(..., timeout=...)) and update all DB methods (including
_create_tables, any read/write methods) to use a context manager like with
self._get_connection() as conn: instead of using self.conn; this ensures each
operation gets its own connection (or you can alternatively add a threading.Lock
around writes if preferred).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3fe68448-7676-4919-94b9-e6694de8f17e
📒 Files selected for processing (2)
src/db_storage.pysrc/main.py
Comment on lines
+931
to
+942
| # Cancel cleanup task on shutdown | ||
| async def post_shutdown(app): # pylint: disable=unused-argument | ||
| global cleanup_task # pylint: disable=global-statement | ||
| if cleanup_task is not None: | ||
| cleanup_task.cancel() | ||
| try: | ||
| await cleanup_task | ||
| except asyncio.CancelledError: | ||
| pass | ||
|
|
||
| application.post_init = post_init | ||
| application.post_shutdown = post_shutdown |
Contributor
There was a problem hiding this comment.
Database connection not closed on shutdown.
The post_shutdown hook cancels the cleanup task but doesn't close the database connection. This may leave the SQLite connection open.
🛠️ Proposed fix
async def post_shutdown(app): # pylint: disable=unused-argument
global cleanup_task # pylint: disable=global-statement
if cleanup_task is not None:
cleanup_task.cancel()
try:
await cleanup_task
except asyncio.CancelledError:
pass
+ # Close database connection
+ db_storage.close()
application.post_init = post_init
application.post_shutdown = post_shutdown🧰 Tools
🪛 Ruff (0.15.2)
[warning] 932-932: Unused function argument: app
(ARG001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main.py` around lines 931 - 942, post_shutdown currently only cancels
cleanup_task and never closes the SQLite DB; update post_shutdown to also close
the application's DB connection: reference the same global DB handle (e.g., the
module-level sqlite connection variable used elsewhere) inside post_shutdown
after cancelling/awaiting cleanup_task, check it's not None, call its close()
method (handling any exceptions like OperationalError) and set the global to
None so the connection is fully released before shutdown; keep the existing
asyncio.CancelledError handling and ensure application.post_shutdown remains
assigned to post_shutdown.
- Close DB connection on shutdown - Initialize response variable before retry loop - Localize Gemini fallback instruction - Move time import to module level in db_storage
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/main.py (1)
795-799:⚠️ Potential issue | 🟠 MajorAvoid
BLOCK_NONEdefaults for all Gemini harm categories in a public chatbot.This fully disables automated harm blocking and materially weakens safety/compliance posture.
Gemini API safety settings documentation: for public user-generated chatbots, what are the recommended HarmBlockThreshold defaults, and what are the implications of using BLOCK_NONE?🛡️ Suggested baseline
safety_settings = { - genai.types.HarmCategory.HARM_CATEGORY_HARASSMENT: genai.types.HarmBlockThreshold.BLOCK_NONE, - genai.types.HarmCategory.HARM_CATEGORY_HATE_SPEECH: genai.types.HarmBlockThreshold.BLOCK_NONE, - genai.types.HarmCategory.HARM_CATEGORY_SEXUALLY_EXPLICIT: genai.types.HarmBlockThreshold.BLOCK_NONE, - genai.types.HarmCategory.HARM_CATEGORY_DANGEROUS_CONTENT: genai.types.HarmBlockThreshold.BLOCK_NONE, + genai.types.HarmCategory.HARM_CATEGORY_HARASSMENT: genai.types.HarmBlockThreshold.BLOCK_ONLY_HIGH, + genai.types.HarmCategory.HARM_CATEGORY_HATE_SPEECH: genai.types.HarmBlockThreshold.BLOCK_ONLY_HIGH, + genai.types.HarmCategory.HARM_CATEGORY_SEXUALLY_EXPLICIT: genai.types.HarmBlockThreshold.BLOCK_ONLY_HIGH, + genai.types.HarmCategory.HARM_CATEGORY_DANGEROUS_CONTENT: genai.types.HarmBlockThreshold.BLOCK_ONLY_HIGH, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.py` around lines 795 - 799, The current harm configuration maps all genai.types.HarmCategory entries to genai.types.HarmBlockThreshold.BLOCK_NONE; update this mapping so public chatbot defaults use stricter thresholds per Gemini guidance: set genai.types.HarmCategory.HARM_CATEGORY_HATE_SPEECH, HARM_CATEGORY_SEXUALLY_EXPLICIT, and HARM_CATEGORY_DANGEROUS_CONTENT to genai.types.HarmBlockThreshold.BLOCK_HIGH (or BLOCK_BLOCK if supported), and set genai.types.HarmCategory.HARM_CATEGORY_HARASSMENT to a non‑NONE threshold such as genai.types.HarmBlockThreshold.BLOCK_WARNING or genai.types.HarmBlockThreshold.BLOCK_MODERATE; replace the BLOCK_NONE values in the mapping in src/main.py accordingly and ensure the mapping variable is used by the request setup so the new defaults take effect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.py`:
- Line 943: Remove the unnecessary global declaration to fix W0602: in the
function main(), delete the line declaring "global cleanup_task" (cleanup_task
is not assigned there); if you need to reference/modify cleanup_task in
post_shutdown(), keep or add "global cleanup_task" inside post_shutdown and
ensure you assign to cleanup_task there (e.g., set cleanup_task = ... before
using it) so the global is actually bound where it's modified.
- Around line 703-721: The db persistence call (db_storage.save_user_data
invoked via asyncio.to_thread) must be made best-effort so a failure does not
turn a successful LLM reply into a user-visible error: send the reply with
update.message.reply_text first, then perform persistence asynchronously and
swallow/log errors; e.g., move or duplicate the
asyncio.to_thread(db_storage.save_user_data, ...) call into a background
try/except (or an asyncio.create_task wrapper) that catches exceptions and logs
them (do not re-raise), so failures in save_user_data or the to_thread call do
not affect the previously sent reply.
- Around line 842-850: call_gemini_api is sending an error reply via
update.message.reply_text and then raising, which leads respond_with_llm_message
to send a second error; remove the user-facing reply from call_gemini_api and
instead raise a descriptive exception (or a custom GeminiRetryError) that
includes the localized fail_msg; then update respond_with_llm_message to catch
that exception and send a single reply using update.message.reply_text (using
the fail_msg from the exception) so only one error message is delivered. Ensure
references to call_gemini_api, respond_with_llm_message, and
update.message.reply_text are updated accordingly and handle both locations
noted (this block and the similar 723-748 path).
- Around line 891-915: The cleanup_stale_users background loop currently can die
on any exception (e.g., transient DB errors); wrap the loop body inside a
try/except that catches Exception around the await asyncio.sleep(...) + db calls
in cleanup_stale_users and ensure any exception is logged (including exception
details) but does not re-raise, so the loop continues; specifically protect
calls to db_storage.get_stale_users and db_storage.delete_user_data and the
in-memory deletions, log failures via the existing logger (include the
exception), and optionally add a short retry/backoff sleep on exception before
continuing the while True loop.
---
Duplicate comments:
In `@src/main.py`:
- Around line 795-799: The current harm configuration maps all
genai.types.HarmCategory entries to genai.types.HarmBlockThreshold.BLOCK_NONE;
update this mapping so public chatbot defaults use stricter thresholds per
Gemini guidance: set genai.types.HarmCategory.HARM_CATEGORY_HATE_SPEECH,
HARM_CATEGORY_SEXUALLY_EXPLICIT, and HARM_CATEGORY_DANGEROUS_CONTENT to
genai.types.HarmBlockThreshold.BLOCK_HIGH (or BLOCK_BLOCK if supported), and set
genai.types.HarmCategory.HARM_CATEGORY_HARASSMENT to a non‑NONE threshold such
as genai.types.HarmBlockThreshold.BLOCK_WARNING or
genai.types.HarmBlockThreshold.BLOCK_MODERATE; replace the BLOCK_NONE values in
the mapping in src/main.py accordingly and ensure the mapping variable is used
by the request setup so the new defaults take effect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f31724e4-46bd-42b8-a539-b814b09077bf
📒 Files selected for processing (2)
src/db_storage.pysrc/main.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/db_storage.py
Comment on lines
+842
to
+850
| # Check if response was set after retries | ||
| if response is None: | ||
| fail_msg = ( | ||
| "Вибачте, не вдалося отримати відповідь. Спробуйте пізніше." | ||
| if language == "uk" | ||
| else "Sorry, I encountered an unexpected error while processing your request." | ||
| else "Sorry, failed to get a response. Please try again later." | ||
| ) | ||
| await update.message.reply_text(fail_msg) | ||
| raise Exception("Failed to get response after retries") # pylint: disable=broad-exception-raised |
Contributor
There was a problem hiding this comment.
Gemini retry exhaustion can send duplicate error messages.
call_gemini_api sends a failure reply and then raises; respond_with_llm_message catches and sends another error. This creates double replies for one failure path.
🧩 Suggested fix
if response is None:
fail_msg = (
"Вибачте, не вдалося отримати відповідь. Спробуйте пізніше."
if language == "uk"
else "Sorry, failed to get a response. Please try again later."
)
- await update.message.reply_text(fail_msg)
- raise Exception("Failed to get response after retries") # pylint: disable=broad-exception-raised
+ raise RuntimeError(fail_msg)Also applies to: 723-748
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 850-850: Create your own exception
(TRY002)
[warning] 850-850: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main.py` around lines 842 - 850, call_gemini_api is sending an error
reply via update.message.reply_text and then raising, which leads
respond_with_llm_message to send a second error; remove the user-facing reply
from call_gemini_api and instead raise a descriptive exception (or a custom
GeminiRetryError) that includes the localized fail_msg; then update
respond_with_llm_message to catch that exception and send a single reply using
update.message.reply_text (using the fail_msg from the exception) so only one
error message is delivered. Ensure references to call_gemini_api,
respond_with_llm_message, and update.message.reply_text are updated accordingly
and handle both locations noted (this block and the similar 723-748 path).
- Make DB persistence best-effort (send reply first) - Remove duplicate error message from call_gemini_api - Protect cleanup loop from crashes with exception handling
ovchynnikov
approved these changes
Mar 11, 2026
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.
Add Grok API support, improve LLM integration, and add SQLite persistence
Summary
This PR adds Grok API as an alternative LLM provider alongside Gemini, implements conversation context tracking with SQLite persistence, adds configurable rate limiting, improves error handling, and optimizes token usage through context truncation.
Changes
1. Grok API Integration
LLM_PROVIDER,GROK_API_KEY,GROK_MODELgemini_*variables tollm_*for provider-agnostic namingopenai>=1.0.0dependency to requirements.txt2. Conversation Context
USE_CONVERSATION_CONTEXTflag (default: True)MAX_CONTEXT_MESSAGESto control number of exchanges stored (default: 3)MAX_CONTEXT_CHARSto limit token usage by truncating stored messages (default: 500 chars)3. SQLite Persistence
db_storage.py- SQLite storage module for persistent user data/bot/data/bot.db(Docker) orsrc/data/bot.db(local)-v bot-data:/bot/data4. Rate Limiting
LLM_RPM_LIMIT(requests per minute, default: 50)LLM_RPD_LIMIT(requests per day, default: 500)5. Memory Management
USER_CLEANUP_TTL_DAYS(default: 3) - days before user data expiresUSER_CLEANUP_INTERVAL_HOURS(default: 24) - cleanup check intervaluser_last_seentimestamps6. Error Handling & Logging
USE_LLM=Truetracebackimport to module levelALLOWED_PROVIDERSset7. Code Quality
# pylint: disable=broad-exception-caughtcomments for retry logic# pylint: disable=unused-argumentfor callback signaturespost_initcallback pattern8. Configuration & Documentation
USE_LLMvariable to.env.example.env.example/bot/datadirectory.gitignoreEnvironment Variables Added
Docker Usage
Benefits
Testing
LOG_LEVEL=DEBUGto see detailed API logs and error tracesUSE_LLM=Trueand bothLLM_PROVIDER=grokandLLM_PROVIDER=geminiRelated Issues
Summary by CodeRabbit
New Features
Improvements
Chores