feat(OCR): 添加基于大模型的OCR作为PaddleOCR的替代方案#243
Merged
Merged
Conversation
…e bundle support - Add EnableLocalBotAPI, TelegramBotApiId, TelegramBotApiHash, LocalBotApiPort to Config/Env - Auto-set BaseUrl and IsLocalAPI when EnableLocalBotAPI is true - Launch telegram-bot-api.exe as a ChildProcessManager-managed process in GeneralBootstrap - Add conditional Content entry for telegram-bot-api.exe in .csproj - Build telegram-bot-api from source via cmake+vcpkg in GitHub Actions push workflow Co-authored-by: ModerRAS <28183976+ModerRAS@users.noreply.github.com>
- 新增IOCRService接口和OCREngine枚举 - 新增LLMOCRService实现，使用GeneralLLMService.AnalyzeImageAsync - 新增OCRConfState枚举和状态机 - 新增EditOCRConfRedisHelper用于Redis状态存储 - 新增EditOCRConfService实现OCR配置状态机逻辑 - 新增EditOCRConfController和EditOCRConfView用于Bot私聊配置 - 修改AutoOCRController支持OCR引擎动态切换 - 修改PaddleOCRService实现IOCRService接口 支持通过Bot私聊发送"OCR设置"进行配置
📝 WalkthroughWalkthroughThis PR implements a pluggable OCR service architecture supporting multiple engines (PaddleOCR and LLM-based), introduces an interactive OCR configuration state machine with Redis-backed persistence, and adds custom prompt support to LLM image analysis. It also enhances local Telegram Bot API startup with explicit TCP port polling. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Telegram
participant Controller as EditOCRConfController
participant Service as EditOCRConfService
participant Redis as Redis
participant DB as EF Core DB
participant View as EditOCRConfView
User->>Controller: /ocrconfig command
Controller->>Service: ExecuteAsync("OCR设置", chatId)
Service->>Redis: GetStateAsync()
Redis-->>Service: null (no active session)
Service->>Redis: SetStateAsync("main_menu")
Service->>DB: Read OCR:Engine config
Service-->>Controller: (true, "Select option")
Controller->>Service: GetReplyMarkupAsync(chatId)
Service->>Redis: GetStateAsync()
Redis-->>Service: "main_menu"
Service-->>Controller: Keyboard with [Switch Engine, View Config, Exit]
Controller->>View: WithMessage(...).WithReplyMarkup(...)
View->>User: Send config menu
User->>Controller: PaddleOCR selection
Controller->>Service: ExecuteAsync("PaddleOCR", chatId)
Service->>Redis: GetStateAsync()
Redis-->>Service: "main_menu"
Service->>DB: Write OCR:Engine = "PaddleOCR"
Service-->>Controller: (true, "Engine updated")
Controller->>View: Render confirmation
View->>User: Send confirmation + new menu
sequenceDiagram
participant User as User/Telegram
participant Controller as AutoOCRController
participant Service as IAppConfigurationService
participant Registry as IOCRService Registry
participant PaddleOCR as PaddleOCRService
participant LLMOCR as LLMOCRService
participant LLMService as IGeneralLLMService
User->>Controller: Send image for OCR
Controller->>Service: GetOCREngineAsync()
Service-->>Controller: OCREngine.LLM
Controller->>Registry: Find service with Engine==LLM
Registry-->>Controller: LLMOCRService
Controller->>LLMOCR: ExecuteAsync(imageStream)
LLMOCR->>LLMOCR: Convert stream to JPEG
LLMOCR->>LLMService: AnalyzeImageAsync(path, DefaultVisionOcrPrompt)
LLMService-->>LLMOCR: Recognized text
LLMOCR-->>Controller: OCR result
Controller->>User: Send recognized text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
🔍 PR检查报告📋 检查概览
🧪 测试结果
📊 代码质量
📁 测试产物
🔗 相关链接此报告由GitHub Actions自动生成 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (4)
TelegramSearchBot/AppBootstrap/GeneralBootstrap.cs (1)
97-131: Windows-only implementation - consider documenting or guarding.This implementation is Windows-specific:
telegram-bot-api.exeis a Windows binarychildProcessManageruses Windows job objects (seeAppBootstrap.cs:19-143)Linux users with
EnableLocalBotAPI = truewill see the warning at line 129 but might be confused. Consider adding platform documentation or a more explicit message.🔧 Suggested improvement for cross-platform clarity
if (Env.EnableLocalBotAPI) { +#if !WINDOWS + Log.Warning("EnableLocalBotAPI is currently only supported on Windows. Feature disabled."); +#else string botApiExePath = Path.Combine(AppContext.BaseDirectory, "telegram-bot-api.exe"); // ... existing code ... +#endif }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/AppBootstrap/GeneralBootstrap.cs` around lines 97 - 131, The current local telegram-bot-api startup block (guarded by Env.EnableLocalBotAPI) assumes a Windows executable and Windows job-object management (see botApiExePath, childProcessManager and WaitForLocalBotApiReady); update the logic to first check the OS (e.g., RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) and only attempt to locate and start "telegram-bot-api.exe" and register it with childProcessManager on Windows, otherwise log a clear platform-specific message that EnableLocalBotAPI is not supported on the current OS and suggest the recommended Linux alternative or documentation link; ensure the log includes which platform was detected and keep the existing Windows startup flow (ArgumentList, AddProcess, WaitForLocalBotApiReady) unchanged when on Windows..github/workflows/push.yml (1)
36-53: Build step lacks explicit error handling for vcpkg/cmake failures.If vcpkg install or cmake fails, the step will fail the entire workflow without clear indication of which part failed. Consider adding explicit error checks or using
$ErrorActionPreference = 'Stop'at the start.Also, the recursive clone of
tdlib/telegram-bot-apican be slow and bandwidth-intensive. The current caching strategy helps, but initial builds or cache misses will be slow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/push.yml around lines 36 - 53, The "Build telegram-bot-api from source" step lacks explicit failure handling and does a full recursive clone; make the step fail-fast and emit clear errors by setting PowerShell to stop on errors (e.g., set $ErrorActionPreference = 'Stop' at the top of the run block) and wrap the key commands (git clone, C:\vcpkg\vcpkg.exe install, cmake configure/build) in try/catch that logs which command failed (include the command name like git clone, vcpkg install, cmake) and rethrows/exit with a non‑zero code; also replace the recursive git clone with a shallow non-recursive clone (e.g., --depth 1 and no --recursive) or explicitly fetch only needed submodules to reduce bandwidth on cache misses.TelegramSearchBot/TelegramSearchBot.csproj (1)
99-103: Cross-platform consideration: Windows-only binary packaging.The project targets both
win-x64andlinux-x64(line 5), but onlytelegram-bot-api.exeis packaged. Linux deployments would needtelegram-bot-api(no.exeextension) built for Linux.The
Condition="Exists('telegram-bot-api.exe')"gracefully handles the missing file case, so Linux builds won't fail. However, users enablingEnableLocalBotAPIon Linux won't have the binary available.Consider documenting this Windows-only limitation or adding Linux binary support in a future iteration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/TelegramSearchBot.csproj` around lines 99 - 103, The project currently packages only the Windows binary "telegram-bot-api.exe" (ItemGroup Content Include="telegram-bot-api.exe") while the runtime identifiers include both "win-x64" and "linux-x64"; update the packaging to account for Linux by either adding a conditional Content entry for the Linux binary name "telegram-bot-api" (and/or a platform-specific MSBuild Condition based on RuntimeIdentifier) or document that EnableLocalBotAPI is Windows-only so Linux users know they must provide a Linux build; locate the ItemGroup that references "telegram-bot-api.exe" and adjust it to include the Linux artifact or add clear documentation referencing EnableLocalBotAPI and the runtime identifiers.TelegramSearchBot/Service/Manage/EditOCRConfService.cs (1)
215-234: Persist LLM channel/model atomically to avoid partial configuration
SetLLMConfigAsyncperforms two separate writes, each with its ownSaveChangesAsync. If the second write fails, config can become partially updated.💡 Suggested fix
private async Task SetLLMConfigAsync(int channelId, string modelName) { - await SetConfigAsync(OCRLLMChannelIdKey, channelId.ToString()); - await SetConfigAsync(OCRLLMModelNameKey, modelName); + await UpsertConfigEntityAsync(OCRLLMChannelIdKey, channelId.ToString()); + await UpsertConfigEntityAsync(OCRLLMModelNameKey, modelName); + await DataContext.SaveChangesAsync(); } -private async Task SetConfigAsync(string key, string value) { +private async Task UpsertConfigEntityAsync(string key, string value) { var config = await DataContext.AppConfigurationItems .FirstOrDefaultAsync(x => x.Key == key); if (config == null) { await DataContext.AppConfigurationItems.AddAsync(new AppConfigurationItem { Key = key, Value = value }); } else { config.Value = value; } - - await DataContext.SaveChangesAsync(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/Manage/EditOCRConfService.cs` around lines 215 - 234, SetLLMConfigAsync currently calls SetConfigAsync twice which invokes DataContext.SaveChangesAsync for each key, risking partial updates; change SetLLMConfigAsync to set or upsert both OCRLLMChannelIdKey and OCRLLMModelNameKey on the DataContext.AppConfigurationItems in-memory first and then call a single SaveChangesAsync (or wrap both SetConfigAsync operations in a database transaction using DataContext.Database.BeginTransactionAsync and commit once) so both values are persisted atomically; update or add AppConfigurationItem entries for OCRLLMChannelIdKey and OCRLLMModelNameKey then perform one DataContext.SaveChangesAsync (or commit the transaction) to avoid partial configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/push.yml:
- Around line 54-57: The copy step "Copy telegram-bot-api binary to project"
runs unconditionally but will fail if neither the cache hit nor the build
produced telegram-bot-api-bin\telegram-bot-api.exe; update that job step to only
run when the binary is available by adding an if condition checking the cache
hit or build success (for example: if: steps.cache.outputs.cache-hit == 'true'
|| steps.build.outcome == 'success'), or alternatively check the file existence
before copying (using a conditional script or an if: expression) so the step is
skipped when the binary doesn't exist.
In `@TelegramSearchBot.Common/Env.cs`:
- Around line 86-89: Add documentation entries in the Docs/ folder for the newly
persisted configuration fields added to the Config/Env class: EnableLocalBotAPI,
TelegramBotApiId, TelegramBotApiHash, and LocalBotApiPort. Update the
user-facing configuration reference (e.g., the config schema docs and any
example config files) to describe each field, its type (bool, string, string,
int), default values (false, null/empty, null/empty, 8081), expected usage, and
any security notes for TelegramBotApiId/TelegramBotApiHash; also add a short
migration note so users know these are now persistent. Ensure the docs reference
the same symbols (EnableLocalBotAPI, TelegramBotApiId, TelegramBotApiHash,
LocalBotApiPort) so they match the code.
In `@TelegramSearchBot/AppBootstrap/GeneralBootstrap.cs`:
- Around line 108-126: The ProcessStartInfo currently sets
RedirectStandardOutput and RedirectStandardError to true but never consumes
those streams, which can cause the child process to hang; fix by either
disabling redirection (set RedirectStandardOutput = false and
RedirectStandardError = false on the ProcessStartInfo used to start the bot API)
or, if you need logs, consume the streams asynchronously after Process.Start by
attaching asynchronous readers to botApiProcess.StandardOutput and
botApiProcess.StandardError (or use BeginOutputReadLine/BeginErrorReadLine or
Task-based reads) and forward them to Log before calling
childProcessManager.AddProcess and awaiting WaitForLocalBotApiReady so buffers
never fill.
In `@TelegramSearchBot/Controller/AI/OCR/AutoOCRController.cs`:
- Around line 74-79: The controller currently awaits LLM OCR inline
(GetOCREngineAsync -> selecting _ocrServices where Engine == OCREngine.LLM and
calling LLMOCRService.ExecuteAsync), causing long-running work inside the
IOnUpdate path; change the flow so the controller does not run LLM OCR
synchronously: detect when GetOCREngineAsync returns OCREngine.LLM and instead
enqueue or dispatch the image to the existing subprocess/scheduler service used
for PaddleOCR (or the AppBootstrap-launched OCR subprocess) rather than calling
LLMOCRService.ExecuteAsync directly; keep synchronous/fast handling for update
handlers by selecting the local OCR service (e.g., PaddleOCR) for immediate
ExecuteAsync and push LLM requests to the background scheduler/service (use the
project’s scheduler enqueue method or subprocess launcher used elsewhere) so
IOnUpdate never performs image conversion/temp-file I/O or LLM network calls
inline.
In `@TelegramSearchBot/Controller/Manage/EditOCRConfController.cs`:
- Around line 18-20: EditOCRConfController currently holds a mutable
EditOCRConfView instance that gets reused by ControllerExecutor, causing
cross-request state corruption; remove the view field from EditOCRConfController
and either (A) change EditOCRConfView to be stateless by replacing instance
fields/_chatId/_replyToMessageId/_messageText with a Render(chatId,
replyToMessageId, messageText) method, or (B) instantiate a new EditOCRConfView
inside the controller execution path each request before calling its render/send
methods; update all places in EditOCRConfController that read or mutate
EditOCRConfView (e.g., where you set chatId/replyToMessageId/messageText) to use
the stateless Render signature or the fresh instance, ensuring the controller
remains stateless as required by ControllerExecutor.
In `@TelegramSearchBot/Helper/EditOCRConfRedisHelper.cs`:
- Around line 30-31: The Redis keys for the transient edit flow are never
expiring; update SetStateAsync (and the other write methods referenced around
lines 38-39 and 50-51) to set a short TTL and refresh it on every write:
introduce a single TTL constant (e.g., TimeSpan editFlowTtl =
TimeSpan.FromMinutes(10)) in the class and use GetDatabase().StringSetAsync(key,
value, editFlowTtl) (or call KeyExpireAsync immediately after StringSetAsync)
for StateKey and the other keys written by SetSourceAsync / SetDocIdAsync so
each write resets the expiry and stale keys are automatically removed.
- Around line 42-51: Change the channel ID methods and storage to use long
instead of int: update GetChannelIdAsync to parse the stored string with
long.TryParse and return a long? (nullable long) and update SetChannelIdAsync to
accept a long channelId and store channelId.ToString(); ensure you reference
GetChannelIdAsync, SetChannelIdAsync and ChannelKey (and align with the existing
_chatId constructor parameter) so the Redis read/write uses 64-bit values and
preserves negative -100... Telegram IDs.
In `@TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs`:
- Around line 37-38: The call to _generalLLMService.AnalyzeImageAsync(tempPath,
0) discards the originating chat context; update IOCRService.ExecuteAsync to
accept a chatId parameter (thread e.Message.Chat.Id from the caller), propagate
that chatId into LLMOCRService (or wherever ExecuteAsync is implemented), and
replace the hardcoded 0 with the passed chatId when calling
_generalLLMService.AnalyzeImageAsync(tempPath, chatId) so chat-scoped
routing/configuration in GeneralLLMService receives the correct ChatId.
- Around line 30-35: The current use of Path.GetTempFileName() + ".jpg" creates
an unused .tmp file each request; change the tempPath generation in
LLMOCRService (tempPath variable) to create a unique JPG path without
pre-creating a file (e.g., Path.Combine(Path.GetTempPath(),
Guid.NewGuid().ToString() + ".jpg") or Path.GetRandomFileName() + ".jpg"), write
the image to that path in your existing FileStream block, and ensure you delete
the tempPath in a finally/cleanup block after processing to avoid leaking temp
files.
In `@TelegramSearchBot/Service/Manage/EditOCRConfService.cs`:
- Around line 60-63: The handler currently treats both "退出" and "返回" the same
and clears state via redis.DeleteKeysAsync(), which conflicts with the UI prompt
that says 输入`返回`返回主菜单; update the logic so only the "退出" command triggers
redis.DeleteKeysAsync() and returns the exit message, while "返回" should return
(true, "返回主菜单") or equivalent without clearing state; adjust any related
prompt/response text (the view prompt that mentions 输入`返回`返回主菜单) to remain
consistent with this new behavior.
- Around line 77-91: HandleMainMenuAsync currently ignores the incoming command
and always re-renders the menu; update it to parse the command and set the next
interaction state on the EditOCRConfRedisHelper so the flow can transition
(e.g., set redis.State to SelectingEngine when command == "1", to ViewingConfig
when command == "2", and handle "3" as return/exit). Keep returning the menu
text when command is empty or invalid, but when a valid option is chosen return
the appropriate state change (and a brief confirmation message) so downstream
handlers (SelectingEngine/ViewingConfig) are reachable; reference the
HandleMainMenuAsync method, the redis parameter, and the
SelectingEngine/ViewingConfig state names when making the change.
- Around line 70-75: The code currently returns (false, string.Empty) when
currentState is not found in _stateHandlers, which leaves the conversation
dead-ended; instead reset the conversation state to MainMenu and return a
friendly message. In the else branch where
_stateHandlers.TryGetValue(currentState, out var handler) is false, write the
new state ("MainMenu") back to Redis (or call the existing state setter used
elsewhere), ensure any in-memory state is updated, and return (true, "<brief
prompt for main menu>") or similar so the user is guided back to MainMenu;
reference _stateHandlers, currentState, handler and MainMenu when making the
change.
---
Nitpick comments:
In @.github/workflows/push.yml:
- Around line 36-53: The "Build telegram-bot-api from source" step lacks
explicit failure handling and does a full recursive clone; make the step
fail-fast and emit clear errors by setting PowerShell to stop on errors (e.g.,
set $ErrorActionPreference = 'Stop' at the top of the run block) and wrap the
key commands (git clone, C:\vcpkg\vcpkg.exe install, cmake configure/build) in
try/catch that logs which command failed (include the command name like git
clone, vcpkg install, cmake) and rethrows/exit with a non‑zero code; also
replace the recursive git clone with a shallow non-recursive clone (e.g.,
--depth 1 and no --recursive) or explicitly fetch only needed submodules to
reduce bandwidth on cache misses.
In `@TelegramSearchBot/AppBootstrap/GeneralBootstrap.cs`:
- Around line 97-131: The current local telegram-bot-api startup block (guarded
by Env.EnableLocalBotAPI) assumes a Windows executable and Windows job-object
management (see botApiExePath, childProcessManager and WaitForLocalBotApiReady);
update the logic to first check the OS (e.g.,
RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) and only attempt to locate
and start "telegram-bot-api.exe" and register it with childProcessManager on
Windows, otherwise log a clear platform-specific message that EnableLocalBotAPI
is not supported on the current OS and suggest the recommended Linux alternative
or documentation link; ensure the log includes which platform was detected and
keep the existing Windows startup flow (ArgumentList, AddProcess,
WaitForLocalBotApiReady) unchanged when on Windows.
In `@TelegramSearchBot/Service/Manage/EditOCRConfService.cs`:
- Around line 215-234: SetLLMConfigAsync currently calls SetConfigAsync twice
which invokes DataContext.SaveChangesAsync for each key, risking partial
updates; change SetLLMConfigAsync to set or upsert both OCRLLMChannelIdKey and
OCRLLMModelNameKey on the DataContext.AppConfigurationItems in-memory first and
then call a single SaveChangesAsync (or wrap both SetConfigAsync operations in a
database transaction using DataContext.Database.BeginTransactionAsync and commit
once) so both values are persisted atomically; update or add
AppConfigurationItem entries for OCRLLMChannelIdKey and OCRLLMModelNameKey then
perform one DataContext.SaveChangesAsync (or commit the transaction) to avoid
partial configuration.
In `@TelegramSearchBot/TelegramSearchBot.csproj`:
- Around line 99-103: The project currently packages only the Windows binary
"telegram-bot-api.exe" (ItemGroup Content Include="telegram-bot-api.exe") while
the runtime identifiers include both "win-x64" and "linux-x64"; update the
packaging to account for Linux by either adding a conditional Content entry for
the Linux binary name "telegram-bot-api" (and/or a platform-specific MSBuild
Condition based on RuntimeIdentifier) or document that EnableLocalBotAPI is
Windows-only so Linux users know they must provide a Linux build; locate the
ItemGroup that references "telegram-bot-api.exe" and adjust it to include the
Linux artifact or add clear documentation referencing EnableLocalBotAPI and the
runtime identifiers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8202ab74-6587-4b38-8fed-2af0e43f4739
📒 Files selected for processing (13)
.github/workflows/push.ymlTelegramSearchBot.Common/Env.csTelegramSearchBot/AppBootstrap/GeneralBootstrap.csTelegramSearchBot/Controller/AI/OCR/AutoOCRController.csTelegramSearchBot/Controller/Manage/EditOCRConfController.csTelegramSearchBot/Helper/EditOCRConfRedisHelper.csTelegramSearchBot/Interface/AI/OCR/IOCRService.csTelegramSearchBot/Model/AI/OCRConfState.csTelegramSearchBot/Service/AI/OCR/LLMOCRService.csTelegramSearchBot/Service/AI/OCR/PaddleOCRService.csTelegramSearchBot/Service/Manage/EditOCRConfService.csTelegramSearchBot/TelegramSearchBot.csprojTelegramSearchBot/View/EditOCRConfView.cs
Comment on lines
+54
to
+57
| - name: Copy telegram-bot-api binary to project | ||
| shell: pwsh | ||
| run: | | ||
| Copy-Item "telegram-bot-api-bin\telegram-bot-api.exe" "TelegramSearchBot\telegram-bot-api.exe" |
There was a problem hiding this comment.
Copy step may fail if both cache miss and build fail.
The copy step runs unconditionally, but telegram-bot-api-bin\telegram-bot-api.exe only exists if the cache was hit OR the build succeeded. If the build step fails (e.g., network issue cloning, CMake error), this step will fail the workflow.
Consider adding an if condition or combining with the build step.
🔧 Proposed fix
- name: Copy telegram-bot-api binary to project
+ if: success()
shell: pwsh
run: |
+ if (Test-Path "telegram-bot-api-bin\telegram-bot-api.exe") {
Copy-Item "telegram-bot-api-bin\telegram-bot-api.exe" "TelegramSearchBot\telegram-bot-api.exe"
+ } else {
+ Write-Warning "telegram-bot-api.exe not found, skipping copy"
+ }📝 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.
Suggested change
| - name: Copy telegram-bot-api binary to project | |
| shell: pwsh | |
| run: | | |
| Copy-Item "telegram-bot-api-bin\telegram-bot-api.exe" "TelegramSearchBot\telegram-bot-api.exe" | |
| - name: Copy telegram-bot-api binary to project | |
| if: success() | |
| shell: pwsh | |
| run: | | |
| if (Test-Path "telegram-bot-api-bin\telegram-bot-api.exe") { | |
| Copy-Item "telegram-bot-api-bin\telegram-bot-api.exe" "TelegramSearchBot\telegram-bot-api.exe" | |
| } else { | |
| Write-Warning "telegram-bot-api.exe not found, skipping copy" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/push.yml around lines 54 - 57, The copy step "Copy
telegram-bot-api binary to project" runs unconditionally but will fail if
neither the cache hit nor the build produced
telegram-bot-api-bin\telegram-bot-api.exe; update that job step to only run when
the binary is available by adding an if condition checking the cache hit or
build success (for example: if: steps.cache.outputs.cache-hit == 'true' ||
steps.build.outcome == 'success'), or alternatively check the file existence
before copying (using a conditional script or an if: expression) so the step is
skipped when the binary doesn't exist.
Comment on lines
+86
to
+89
| public bool EnableLocalBotAPI { get; set; } = false; | ||
| public string TelegramBotApiId { get; set; } | ||
| public string TelegramBotApiHash { get; set; } | ||
| public int LocalBotApiPort { get; set; } = 8081; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if documentation exists and mentions these new config options
fd -t f -e md . Docs/ 2>/dev/null | head -5
rg -l "EnableLocalBotAPI|TelegramBotApiId|TelegramBotApiHash|LocalBotApiPort" Docs/ 2>/dev/null || echo "No documentation found for new config fields"Repository: ModerRAS/TelegramSearchBot
Length of output: 268
Update documentation in Docs/ for new configuration fields.
New persistent configuration fields (EnableLocalBotAPI, TelegramBotApiId, TelegramBotApiHash, LocalBotApiPort) have been added to the Config class but are not documented. Per coding guidelines, user documentation must be updated when modifying persistent configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot.Common/Env.cs` around lines 86 - 89, Add documentation
entries in the Docs/ folder for the newly persisted configuration fields added
to the Config/Env class: EnableLocalBotAPI, TelegramBotApiId,
TelegramBotApiHash, and LocalBotApiPort. Update the user-facing configuration
reference (e.g., the config schema docs and any example config files) to
describe each field, its type (bool, string, string, int), default values
(false, null/empty, null/empty, 8081), expected usage, and any security notes
for TelegramBotApiId/TelegramBotApiHash; also add a short migration note so
users know these are now persistent. Ensure the docs reference the same symbols
(EnableLocalBotAPI, TelegramBotApiId, TelegramBotApiHash, LocalBotApiPort) so
they match the code.
Comment on lines
+108
to
+126
| var startInfo = new ProcessStartInfo { | ||
| FileName = botApiExePath, | ||
| UseShellExecute = false, | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true | ||
| }; | ||
| startInfo.ArgumentList.Add("--local"); | ||
| startInfo.ArgumentList.Add($"--api-id={Env.TelegramBotApiId}"); | ||
| startInfo.ArgumentList.Add($"--api-hash={Env.TelegramBotApiHash}"); | ||
| startInfo.ArgumentList.Add($"--dir={botApiDataDir}"); | ||
| startInfo.ArgumentList.Add($"--http-port={Env.LocalBotApiPort}"); | ||
| var botApiProcess = Process.Start(startInfo); | ||
| if (botApiProcess == null) { | ||
| Log.Warning("telegram-bot-api 进程启动失败"); | ||
| } else { | ||
| childProcessManager.AddProcess(botApiProcess); | ||
| Log.Information("telegram-bot-api 已启动，等待端口 {Port} 就绪...", Env.LocalBotApiPort); | ||
| await WaitForLocalBotApiReady(Env.LocalBotApiPort); | ||
| } |
There was a problem hiding this comment.
Redirected stdout/stderr are never consumed - potential process hang.
When RedirectStandardOutput and RedirectStandardError are set to true, the output streams must be read. If the internal buffer fills up (typically 4KB), the child process will block on writes, potentially causing a hang.
Either disable redirection (if you don't need the output) or consume the streams asynchronously.
🔧 Option 1: Disable redirection (simplest)
var startInfo = new ProcessStartInfo {
FileName = botApiExePath,
UseShellExecute = false,
- RedirectStandardOutput = true,
- RedirectStandardError = true
+ RedirectStandardOutput = false,
+ RedirectStandardError = false
};🔧 Option 2: Consume streams asynchronously
var botApiProcess = Process.Start(startInfo);
if (botApiProcess == null) {
Log.Warning("telegram-bot-api 进程启动失败");
} else {
+ // Consume stdout/stderr to prevent buffer blocking
+ botApiProcess.OutputDataReceived += (s, e) => { if (e.Data != null) Log.Debug("[bot-api stdout] {Line}", e.Data); };
+ botApiProcess.ErrorDataReceived += (s, e) => { if (e.Data != null) Log.Debug("[bot-api stderr] {Line}", e.Data); };
+ botApiProcess.BeginOutputReadLine();
+ botApiProcess.BeginErrorReadLine();
childProcessManager.AddProcess(botApiProcess);📝 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.
Suggested change
| var startInfo = new ProcessStartInfo { | |
| FileName = botApiExePath, | |
| UseShellExecute = false, | |
| RedirectStandardOutput = true, | |
| RedirectStandardError = true | |
| }; | |
| startInfo.ArgumentList.Add("--local"); | |
| startInfo.ArgumentList.Add($"--api-id={Env.TelegramBotApiId}"); | |
| startInfo.ArgumentList.Add($"--api-hash={Env.TelegramBotApiHash}"); | |
| startInfo.ArgumentList.Add($"--dir={botApiDataDir}"); | |
| startInfo.ArgumentList.Add($"--http-port={Env.LocalBotApiPort}"); | |
| var botApiProcess = Process.Start(startInfo); | |
| if (botApiProcess == null) { | |
| Log.Warning("telegram-bot-api 进程启动失败"); | |
| } else { | |
| childProcessManager.AddProcess(botApiProcess); | |
| Log.Information("telegram-bot-api 已启动，等待端口 {Port} 就绪...", Env.LocalBotApiPort); | |
| await WaitForLocalBotApiReady(Env.LocalBotApiPort); | |
| } | |
| var startInfo = new ProcessStartInfo { | |
| FileName = botApiExePath, | |
| UseShellExecute = false, | |
| RedirectStandardOutput = false, | |
| RedirectStandardError = false | |
| }; | |
| startInfo.ArgumentList.Add("--local"); | |
| startInfo.ArgumentList.Add($"--api-id={Env.TelegramBotApiId}"); | |
| startInfo.ArgumentList.Add($"--api-hash={Env.TelegramBotApiHash}"); | |
| startInfo.ArgumentList.Add($"--dir={botApiDataDir}"); | |
| startInfo.ArgumentList.Add($"--http-port={Env.LocalBotApiPort}"); | |
| var botApiProcess = Process.Start(startInfo); | |
| if (botApiProcess == null) { | |
| Log.Warning("telegram-bot-api 进程启动失败"); | |
| } else { | |
| childProcessManager.AddProcess(botApiProcess); | |
| Log.Information("telegram-bot-api 已启动，等待端口 {Port} 就绪...", Env.LocalBotApiPort); | |
| await WaitForLocalBotApiReady(Env.LocalBotApiPort); | |
| } |
Suggested change
| var startInfo = new ProcessStartInfo { | |
| FileName = botApiExePath, | |
| UseShellExecute = false, | |
| RedirectStandardOutput = true, | |
| RedirectStandardError = true | |
| }; | |
| startInfo.ArgumentList.Add("--local"); | |
| startInfo.ArgumentList.Add($"--api-id={Env.TelegramBotApiId}"); | |
| startInfo.ArgumentList.Add($"--api-hash={Env.TelegramBotApiHash}"); | |
| startInfo.ArgumentList.Add($"--dir={botApiDataDir}"); | |
| startInfo.ArgumentList.Add($"--http-port={Env.LocalBotApiPort}"); | |
| var botApiProcess = Process.Start(startInfo); | |
| if (botApiProcess == null) { | |
| Log.Warning("telegram-bot-api 进程启动失败"); | |
| } else { | |
| childProcessManager.AddProcess(botApiProcess); | |
| Log.Information("telegram-bot-api 已启动，等待端口 {Port} 就绪...", Env.LocalBotApiPort); | |
| await WaitForLocalBotApiReady(Env.LocalBotApiPort); | |
| } | |
| var startInfo = new ProcessStartInfo { | |
| FileName = botApiExePath, | |
| UseShellExecute = false, | |
| RedirectStandardOutput = true, | |
| RedirectStandardError = true | |
| }; | |
| startInfo.ArgumentList.Add("--local"); | |
| startInfo.ArgumentList.Add($"--api-id={Env.TelegramBotApiId}"); | |
| startInfo.ArgumentList.Add($"--api-hash={Env.TelegramBotApiHash}"); | |
| startInfo.ArgumentList.Add($"--dir={botApiDataDir}"); | |
| startInfo.ArgumentList.Add($"--http-port={Env.LocalBotApiPort}"); | |
| var botApiProcess = Process.Start(startInfo); | |
| if (botApiProcess == null) { | |
| Log.Warning("telegram-bot-api 进程启动失败"); | |
| } else { | |
| // Consume stdout/stderr to prevent buffer blocking | |
| botApiProcess.OutputDataReceived += (s, e) => { if (e.Data != null) Log.Debug("[bot-api stdout] {Line}", e.Data); }; | |
| botApiProcess.ErrorDataReceived += (s, e) => { if (e.Data != null) Log.Debug("[bot-api stderr] {Line}", e.Data); }; | |
| botApiProcess.BeginOutputReadLine(); | |
| botApiProcess.BeginErrorReadLine(); | |
| childProcessManager.AddProcess(botApiProcess); | |
| Log.Information("telegram-bot-api 已启动，等待端口 {Port} 就绪...", Env.LocalBotApiPort); | |
| await WaitForLocalBotApiReady(Env.LocalBotApiPort); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/AppBootstrap/GeneralBootstrap.cs` around lines 108 - 126,
The ProcessStartInfo currently sets RedirectStandardOutput and
RedirectStandardError to true but never consumes those streams, which can cause
the child process to hang; fix by either disabling redirection (set
RedirectStandardOutput = false and RedirectStandardError = false on the
ProcessStartInfo used to start the bot API) or, if you need logs, consume the
streams asynchronously after Process.Start by attaching asynchronous readers to
botApiProcess.StandardOutput and botApiProcess.StandardError (or use
BeginOutputReadLine/BeginErrorReadLine or Task-based reads) and forward them to
Log before calling childProcessManager.AddProcess and awaiting
WaitForLocalBotApiReady so buffers never fill.
Comment on lines
+74
to
+79
| var engine = await GetOCREngineAsync(); | ||
| var ocrService = _ocrServices.FirstOrDefault(s => s.Engine == engine) | ||
| ?? _ocrServices.First(s => s.Engine == OCREngine.PaddleOCR); | ||
|
|
||
| logger.LogInformation($"使用OCR引擎: {engine}"); | ||
| OcrStr = await ocrService.ExecuteAsync(new MemoryStream(PhotoStream)); |
There was a problem hiding this comment.
Keep LLM OCR off the update-handler path.
With OCREngine.LLM, this controller now awaits LLMOCRService.ExecuteAsync(...), which does image conversion, temp-file I/O, and the LLM call inline. That makes ExecuteAsync a long-running handler and can stall the controller pipeline for other updates. Dispatch the LLM OCR through the same subprocess/scheduler boundary as PaddleOCR instead of doing it inside IOnUpdate. As per coding guidelines, "OCR, ASR, and QR code processing must be delegated to independent subprocess services launched via AppBootstrap" and "Long-running operations like OCR and downloads must be placed in Service/Scheduler or separate processes, not in IOnUpdate handlers".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/Controller/AI/OCR/AutoOCRController.cs` around lines 74 -
79, The controller currently awaits LLM OCR inline (GetOCREngineAsync ->
selecting _ocrServices where Engine == OCREngine.LLM and calling
LLMOCRService.ExecuteAsync), causing long-running work inside the IOnUpdate
path; change the flow so the controller does not run LLM OCR synchronously:
detect when GetOCREngineAsync returns OCREngine.LLM and instead enqueue or
dispatch the image to the existing subprocess/scheduler service used for
PaddleOCR (or the AppBootstrap-launched OCR subprocess) rather than calling
LLMOCRService.ExecuteAsync directly; keep synchronous/fast handling for update
handlers by selecting the local OCR service (e.g., PaddleOCR) for immediate
ExecuteAsync and push LLM requests to the background scheduler/service (use the
project’s scheduler enqueue method or subprocess launcher used elsewhere) so
IOnUpdate never performs image conversion/temp-file I/O or LLM network calls
inline.
Comment on lines
+18
to
+20
| protected readonly AdminService AdminService; | ||
| protected readonly EditOCRConfService EditOCRConfService; | ||
| protected readonly EditOCRConfView EditOCRConfView; |
There was a problem hiding this comment.
Don't keep a mutable view instance on the controller.
TelegramSearchBot/Executor/ControllerExecutor.cs:19-37 reuses controller instances, but this controller stores EditOCRConfView as a field and mutates its _chatId/_replyToMessageId/_messageText per request. Two overlapping admin updates can overwrite each other and send the response to the wrong chat/message. Make the view stateless (Render(chatId, replyTo, message)) or create a fresh instance per execution. As per coding guidelines, "Controllers must be stateless; delegate all persistent operations to Service layer classes".
Also applies to: 22-30, 49-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/Controller/Manage/EditOCRConfController.cs` around lines 18
- 20, EditOCRConfController currently holds a mutable EditOCRConfView instance
that gets reused by ControllerExecutor, causing cross-request state corruption;
remove the view field from EditOCRConfController and either (A) change
EditOCRConfView to be stateless by replacing instance
fields/_chatId/_replyToMessageId/_messageText with a Render(chatId,
replyToMessageId, messageText) method, or (B) instantiate a new EditOCRConfView
inside the controller execution path each request before calling its render/send
methods; update all places in EditOCRConfController that read or mutate
EditOCRConfView (e.g., where you set chatId/replyToMessageId/messageText) to use
the stateless Render signature or the fresh instance, ensuring the controller
remains stateless as required by ControllerExecutor.
Comment on lines
+30
to
+35
| var tempPath = Path.GetTempFileName() + ".jpg"; | ||
|
|
||
| try { | ||
| using (var fs = new FileStream(tempPath, FileMode.Create, FileAccess.Write)) { | ||
| tg_img_data.SaveTo(fs); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In .NET, does Path.GetTempFileName() create a file on disk, and what is the recommended way to generate a temporary filename with a custom .jpg extension without leaving the original temp file behind?
💡 Result:
Yes, Path.GetTempFileName creates a uniquely named, zero-byte temporary file on disk with a .tmp extension in the user's temporary folder (Path.GetTempPath). The recommended way to generate a temporary filename with a custom .jpg extension without creating the original temp file is to use Path.GetTempPath combined with Path.GetRandomFileName (which generates a random filename string without creating a file) and change the extension to .jpg: string tempJpgPath = Path.Combine(Path.GetTempPath, Path.ChangeExtension(Path.GetRandomFileName, ".jpg")); This produces a cryptographically strong random name (e.g., "a1b2c3d4.e5f6.jpg") that is statistically unique, avoiding collisions and unnecessary file creation. When ready to use it, create the file yourself (e.g., via File.Create) and delete it after use, as temp files are not auto-deleted. Alternative using GUID (slightly less random but unique): string tempJpgPath = Path.Combine(Path.GetTempPath, Guid.NewGuid.ToString + ".jpg");
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.io.path.gettempfilename?view=net-7.0
- 2: https://learn.microsoft.com/en-us/dotnet/api/system.io.path.gettempfilename?view=net-10.0
- 3: https://evotec.xyz/difference-between-gettempfilename-and-getrandomfilename-that-got-my-ass-kicked/
- 4: https://learn.microsoft.com/en-us/dotnet/api/system.io.path.getrandomfilename?view=net-8.0
- 5: https://learn.microsoft.com/en-us/dotnet/api/system.io.path.gettempfilename?view=net-8.0
- 6: https://stackoverflow.com/questions/581570/how-can-i-create-a-temp-file-with-a-specific-extension-with-net
- 7: https://www.alwaysdeveloping.net/dailydrop/2022/05/19-temp-file-name/
🏁 Script executed:
cat -n TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs | head -50Repository: ModerRAS/TelegramSearchBot
Length of output: 2305
Don't leak a temp file on every OCR request.
Path.GetTempFileName() creates a file on disk immediately. Appending .jpg to that path writes the image to a separate file while leaving the original .tmp file behind, steadily growing the temp directory under load.
Fix
- var tempPath = Path.GetTempFileName() + ".jpg";
+ var tempPath = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.jpg");📝 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.
Suggested change
| var tempPath = Path.GetTempFileName() + ".jpg"; | |
| try { | |
| using (var fs = new FileStream(tempPath, FileMode.Create, FileAccess.Write)) { | |
| tg_img_data.SaveTo(fs); | |
| } | |
| var tempPath = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.jpg"); | |
| try { | |
| using (var fs = new FileStream(tempPath, FileMode.Create, FileAccess.Write)) { | |
| tg_img_data.SaveTo(fs); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs` around lines 30 - 35, The
current use of Path.GetTempFileName() + ".jpg" creates an unused .tmp file each
request; change the tempPath generation in LLMOCRService (tempPath variable) to
create a unique JPG path without pre-creating a file (e.g.,
Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString() + ".jpg") or
Path.GetRandomFileName() + ".jpg"), write the image to that path in your
existing FileStream block, and ensure you delete the tempPath in a
finally/cleanup block after processing to avoid leaking temp files.
Comment on lines
+37
to
+38
| _logger.LogInformation("正在使用LLM进行OCR识别..."); | ||
| var result = await _generalLLMService.AnalyzeImageAsync(tempPath, 0); |
There was a problem hiding this comment.
Pass the real chat id into the LLM call.
TelegramSearchBot/Interface/AI/LLM/IGeneralLLMService.cs:23-24 defines the second AnalyzeImageAsync argument as ChatId. Hardcoding 0 here drops the originating chat context, so any chat-scoped LLM routing/configuration inside GeneralLLMService is bypassed. Please thread e.Message.Chat.Id through IOCRService.ExecuteAsync(...) and use it here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs` around lines 37 - 38, The
call to _generalLLMService.AnalyzeImageAsync(tempPath, 0) discards the
originating chat context; update IOCRService.ExecuteAsync to accept a chatId
parameter (thread e.Message.Chat.Id from the caller), propagate that chatId into
LLMOCRService (or wherever ExecuteAsync is implemented), and replace the
hardcoded 0 with the passed chatId when calling
_generalLLMService.AnalyzeImageAsync(tempPath, chatId) so chat-scoped
routing/configuration in GeneralLLMService receives the correct ChatId.
Comment on lines
+70
to
+75
| if (_stateHandlers.TryGetValue(currentState, out var handler)) { | ||
| return await handler(redis, command); | ||
| } | ||
|
|
||
| return (false, string.Empty); | ||
| } |
There was a problem hiding this comment.
Unknown Redis state should self-recover instead of returning empty response
At Line 74, returning (false, string.Empty) on an unrecognized state gives no user guidance and can dead-end the conversation. Resetting to MainMenu is safer.
💡 Suggested fix
if (_stateHandlers.TryGetValue(currentState, out var handler)) {
return await handler(redis, command);
}
-return (false, string.Empty);
+await redis.SetStateAsync(OCRConfState.MainMenu.GetDescription());
+return await HandleMainMenuAsync(redis, command);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/Service/Manage/EditOCRConfService.cs` around lines 70 - 75,
The code currently returns (false, string.Empty) when currentState is not found
in _stateHandlers, which leaves the conversation dead-ended; instead reset the
conversation state to MainMenu and return a friendly message. In the else branch
where _stateHandlers.TryGetValue(currentState, out var handler) is false, write
the new state ("MainMenu") back to Redis (or call the existing state setter used
elsewhere), ensure any in-memory state is updated, and return (true, "<brief
prompt for main menu>") or similar so the user is guided back to MainMenu;
reference _stateHandlers, currentState, handler and MainMenu when making the
change.
added 2 commits
March 19, 2026 12:26
Fix whitespace formatting issues detected by CI on Windows
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
TelegramSearchBot/Controller/AI/OCR/AutoOCRController.cs (1)
74-79:⚠️ Potential issue | 🟠 MajorKeep LLM OCR off the
IOnUpdatepath.When
GetOCREngineAsync()resolves toOCREngine.LLM, this handler now awaits the full OCR pipeline inline. That puts image conversion, temp-file I/O, and the LLM call on the update path and can stall other updates. Route the LLM branch through the existing subprocess/scheduler boundary instead of callingExecuteAsync()here.As per coding guidelines, "OCR, ASR, and QR code processing must be delegated to independent subprocess services launched via AppBootstrap" and "Long-running operations like OCR and downloads must be placed in Service/Scheduler or separate processes, not in IOnUpdate handlers".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Controller/AI/OCR/AutoOCRController.cs` around lines 74 - 79, The handler currently calls GetOCREngineAsync() and then invokes ocrService.ExecuteAsync inline, which blocks IOnUpdate when engine == OCREngine.LLM; change the flow so that if GetOCREngineAsync() returns OCREngine.LLM you do not call ExecuteAsync here but instead enqueue the OCR job through the existing subprocess/scheduler boundary (use the app's subprocess/service bootstrap or scheduler API used elsewhere) and return immediately, while keeping the existing inline ExecuteAsync behavior only for non-LLM engines; update references around GetOCREngineAsync, OCREngine.LLM, _ocrServices, and ExecuteAsync to implement this routing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@TelegramSearchBot/Controller/AI/OCR/AutoOCRController.cs`:
- Around line 75-78: The code in AutoOCRController is fragile: wrap the config
read (_configService.GetConfigurationValueAsync) in a try/catch and treat
failures as a missing/invalid engine so OCR selection continues; when selecting
the OCR service, use _ocrServices.FirstOrDefault(s => s.Engine == engine) ??
_ocrServices.FirstOrDefault(s => s.Engine == OCREngine.PaddleOCR) and if that
returns null throw a clear InvalidOperationException indicating no OCR
implementations are registered; finally change the log to emit the actual
selected engine (logger.LogInformation($"使用OCR引擎: {ocrService.Engine}")) instead
of the requested value; apply the same changes to the second selection block
around lines 116-121 in AutoOCRController.
---
Duplicate comments:
In `@TelegramSearchBot/Controller/AI/OCR/AutoOCRController.cs`:
- Around line 74-79: The handler currently calls GetOCREngineAsync() and then
invokes ocrService.ExecuteAsync inline, which blocks IOnUpdate when engine ==
OCREngine.LLM; change the flow so that if GetOCREngineAsync() returns
OCREngine.LLM you do not call ExecuteAsync here but instead enqueue the OCR job
through the existing subprocess/scheduler boundary (use the app's
subprocess/service bootstrap or scheduler API used elsewhere) and return
immediately, while keeping the existing inline ExecuteAsync behavior only for
non-LLM engines; update references around GetOCREngineAsync, OCREngine.LLM,
_ocrServices, and ExecuteAsync to implement this routing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68887bb4-edea-4c7d-9ee0-ea1cc6fbb3d3
📒 Files selected for processing (23)
TelegramSearchBot.Common/Attributes/McpAttributes.csTelegramSearchBot.Common/Model/Tools/IterationLimitReachedPayload.csTelegramSearchBot.Database/Migrations/20260303031828_AddUserWithGroupUniqueIndex.csTelegramSearchBot.Database/Migrations/20260313124507_AddChannelWithModelIsDeleted.csTelegramSearchBot.LLM.Test/Service/AI/LLM/GeneralLLMServiceTests.csTelegramSearchBot.LLM.Test/Service/AI/LLM/ModelCapabilityServiceTests.csTelegramSearchBot.LLM.Test/Service/AI/LLM/OpenAIProviderHistorySerializationTests.csTelegramSearchBot.LLM/Service/AI/LLM/GeminiService.csTelegramSearchBot.LLM/Service/AI/LLM/McpToolHelper.csTelegramSearchBot.LLM/Service/AI/LLM/OllamaService.csTelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.csTelegramSearchBot.LLM/Service/Mcp/McpClient.csTelegramSearchBot.LLM/Service/Mcp/McpServerManager.csTelegramSearchBot.LLM/Service/Tools/FileToolService.csTelegramSearchBot.Test/Helper/WordCloudHelperTests.csTelegramSearchBot/Controller/AI/OCR/AutoOCRController.csTelegramSearchBot/Extension/ServiceCollectionExtension.csTelegramSearchBot/Model/AI/OCRConfState.csTelegramSearchBot/Service/AI/OCR/LLMOCRService.csTelegramSearchBot/Service/BotAPI/SendMessageService.Streaming.csTelegramSearchBot/Service/Manage/EditOCRConfService.csTelegramSearchBot/Service/Storage/MessageService.csTelegramSearchBot/Service/Vector/FaissVectorService.cs
✅ Files skipped from review due to trivial changes (20)
- TelegramSearchBot.LLM.Test/Service/AI/LLM/OpenAIProviderHistorySerializationTests.cs
- TelegramSearchBot.Common/Model/Tools/IterationLimitReachedPayload.cs
- TelegramSearchBot.Common/Attributes/McpAttributes.cs
- TelegramSearchBot/Service/BotAPI/SendMessageService.Streaming.cs
- TelegramSearchBot.LLM.Test/Service/AI/LLM/GeneralLLMServiceTests.cs
- TelegramSearchBot/Extension/ServiceCollectionExtension.cs
- TelegramSearchBot.LLM/Service/AI/LLM/GeminiService.cs
- TelegramSearchBot.Database/Migrations/20260303031828_AddUserWithGroupUniqueIndex.cs
- TelegramSearchBot/Service/Vector/FaissVectorService.cs
- TelegramSearchBot.LLM/Service/Mcp/McpClient.cs
- TelegramSearchBot.LLM/Service/Mcp/McpServerManager.cs
- TelegramSearchBot.LLM.Test/Service/AI/LLM/ModelCapabilityServiceTests.cs
- TelegramSearchBot.Database/Migrations/20260313124507_AddChannelWithModelIsDeleted.cs
- TelegramSearchBot.Test/Helper/WordCloudHelperTests.cs
- TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs
- TelegramSearchBot.LLM/Service/Tools/FileToolService.cs
- TelegramSearchBot.LLM/Service/AI/LLM/McpToolHelper.cs
- TelegramSearchBot.LLM/Service/AI/LLM/OllamaService.cs
- TelegramSearchBot/Model/AI/OCRConfState.cs
- TelegramSearchBot/Service/Storage/MessageService.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs
- TelegramSearchBot/Service/Manage/EditOCRConfService.cs
Comment on lines
+75
to
+78
| var ocrService = _ocrServices.FirstOrDefault(s => s.Engine == engine) | ||
| ?? _ocrServices.First(s => s.Engine == OCREngine.PaddleOCR); | ||
|
|
||
| logger.LogInformation($"使用OCR引擎: {engine}"); |
There was a problem hiding this comment.
Harden the fallback path and log the engine actually used.
This only falls back when the stored value is empty or invalid. If _configService.GetConfigurationValueAsync(...) throws, the handler still fails before OCR starts, and Line 76 still throws if PaddleOCR is not registered. In the fallback case, Line 78 also logs the requested engine instead of the selected service. That regresses the PR’s “default PaddleOCR” behavior under transient config or DI issues.
🛠️ Suggested hardening
- var engine = await GetOCREngineAsync();
- var ocrService = _ocrServices.FirstOrDefault(s => s.Engine == engine)
- ?? _ocrServices.First(s => s.Engine == OCREngine.PaddleOCR);
-
- logger.LogInformation($"使用OCR引擎: {engine}");
+ var engine = await GetOCREngineAsync();
+ var ocrService = _ocrServices.FirstOrDefault(s => s.Engine == engine);
+ if (ocrService is null) {
+ logger.LogWarning(
+ "Configured OCR engine {ConfiguredEngine} is unavailable. Falling back to PaddleOCR.",
+ engine);
+ ocrService = _ocrServices.FirstOrDefault(s => s.Engine == OCREngine.PaddleOCR)
+ ?? throw new InvalidOperationException("No PaddleOCR service is registered.");
+ }
+
+ logger.LogInformation("使用OCR引擎: {Engine}", ocrService.Engine);
OcrStr = await ocrService.ExecuteAsync(new MemoryStream(PhotoStream));
@@
- private async Task<OCREngine> GetOCREngineAsync() {
- var engineStr = await _configService.GetConfigurationValueAsync(EditOCRConfService.OCREngineKey);
- if (!string.IsNullOrEmpty(engineStr) && Enum.TryParse<OCREngine>(engineStr, out var engine)) {
- return engine;
- }
- return OCREngine.PaddleOCR;
- }
+ private async Task<OCREngine> GetOCREngineAsync() {
+ try {
+ var engineStr = await _configService.GetConfigurationValueAsync(EditOCRConfService.OCREngineKey);
+ if (!string.IsNullOrEmpty(engineStr) &&
+ Enum.TryParse<OCREngine>(engineStr, ignoreCase: true, out var engine)) {
+ return engine;
+ }
+ } catch (Exception ex) {
+ logger.LogWarning(ex, "Failed to load OCR engine configuration. Falling back to PaddleOCR.");
+ }
+
+ return OCREngine.PaddleOCR;
+ }Also applies to: 116-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/Controller/AI/OCR/AutoOCRController.cs` around lines 75 -
78, The code in AutoOCRController is fragile: wrap the config read
(_configService.GetConfigurationValueAsync) in a try/catch and treat failures as
a missing/invalid engine so OCR selection continues; when selecting the OCR
service, use _ocrServices.FirstOrDefault(s => s.Engine == engine) ??
_ocrServices.FirstOrDefault(s => s.Engine == OCREngine.PaddleOCR) and if that
returns null throw a clear InvalidOperationException indicating no OCR
implementations are registered; finally change the log to emit the actual
selected engine (logger.LogInformation($"使用OCR引擎: {ocrService.Engine}")) instead
of the requested value; apply the same changes to the second selection block
around lines 116-121 in AutoOCRController.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs (2)
39-39:⚠️ Potential issue | 🟠 MajorHardcoded
ChatId = 0drops chat-scoped routing (unresolved).
IGeneralLLMService.AnalyzeImageAsync(PhotoPath, ChatId, prompt, …)usesChatIdfor per-chat model/channel selection. Passing0bypasses that. Thread the real chat id throughIOCRService.ExecuteAsyncfromAutoOCRController(e.Message.Chat.Id) into this call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs` at line 39, The call to _generalLLMService.AnalyzeImageAsync in LLMOCRService (using ChatId = 0) bypasses per-chat routing; instead thread the real chat id from AutoOCRController (use e.Message.Chat.Id) into IOCRService.ExecuteAsync and propagate it to IGeneralLLMService.AnalyzeImageAsync so LLMOCRService calls AnalyzeImageAsync(tempPath, chatId, GeneralLLMService.DefaultVisionOcrPrompt, ...) rather than passing 0; update method signatures (IOCRService.ExecuteAsync and LLMOCRService.ExecuteAsync or related methods) to accept a chatId parameter and pass it through to _generalLLMService.AnalyzeImageAsync.
31-31:⚠️ Potential issue | 🟠 MajorTemp file leak on every OCR request (unresolved).
Path.GetTempFileName()creates a 0-byte.tmpfile on disk and then you append.jpgand write to a different path, leaving the original.tmpbehind forever. Under load the temp directory grows unbounded.- var tempPath = Path.GetTempFileName() + ".jpg"; + var tempPath = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.jpg");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs` at line 31, The code creates a zero-byte .tmp via Path.GetTempFileName() then appends ".jpg" to a different filename (tempPath), leaking the original .tmp; change to create a single temp JPG path and ensure it is deleted after use. Replace Path.GetTempFileName() + ".jpg" with a proper temp JPG like Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N") + ".jpg") (or create the temp file and File.Move to change extension), and add a finally block or using pattern in LLMOCRService to File.Delete(tempPath) (and also delete the original .tmp if you choose File.Move) so no temp files are left behind.
🧹 Nitpick comments (3)
TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs (1)
1238-1286: Prompt wiring LGTM, but pre-existingSKBitmapnot disposed.New
prompthandling is correct. Minor unrelated hygiene:tg_img(SKBitmap) andtg_img_data(SKData) areIDisposableand are leaked on the happy path. Considerusingdeclarations. Not introduced in this PR, optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs` around lines 1238 - 1286, In AnalyzeImageAsync the SKBitmap instance tg_img and the SKData tg_img_data are not disposed on the success path causing a resource leak; wrap tg_img and tg_img_data in using (or using declarations) so both SKBitmap (tg_img) and SKData (tg_img_data) are disposed after Encode/ToArray are called (e.g., using var tg_img = SKBitmap.Decode(fileStream); using var tg_img_data = tg_img.Encode(...)) while keeping fileStream already in its using, and ensure BinaryData.FromBytes still receives the byte[] before disposal.TelegramSearchBot.Test/Service/AI/OCR/LLMOCRServiceTests.cs (1)
14-41: Test does not clean up the temp.jpgleft byLLMOCRService.
LLMOCRService.ExecuteAsyncwrites the encoded JPEG to a temp file and only deletes it in its ownfinallyafter a successful LLM call. Because the mockAnalyzeImageAsyncreturns immediately without the test intercepting the path, the real temp file created byLLMOCRServicewill be cleaned up by the service itself — so this is fine. However, per the project's test guidance (clean up temporary files in test cleanup), consider capturing thepathargument in the mock and asserting it was deleted after the call, to guard against future regressions where an exception path skips cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot.Test/Service/AI/OCR/LLMOCRServiceTests.cs` around lines 14 - 41, The test currently doesn't verify the temp .jpg created by LLMOCRService.ExecuteAsync is deleted; update the mock for IGeneralLLMService.AnalyzeImageAsync to capture the path argument (e.g., via Callback or a local variable) when called with GeneralLLMService.DefaultVisionOcrPrompt, then after awaiting service.ExecuteAsync(stream) assert that File.Exists(capturedPath) is false to ensure the temp file was cleaned up; keep the existing setup/verify for AnalyzeImageAsync and use the capturedPath reference to perform the deletion assertion.TelegramSearchBot.LLM/Service/AI/LLM/GeminiService.cs (1)
375-404: Prompt parameter correctly threaded; pre-existing default-model name is misleading.The new
promptplumbing and fallback toDefaultAltPhotoPromptmatch the interface. Unrelated pre-existing nit: whenmodelNameis empty this service falls back to"gpt-4-vision-preview"(line 377), which is not a valid Gemini model — likely a copy/paste fromOpenAIService. Not introduced by this PR, but worth fixing in a follow-up (e.g.,gemini-1.5-flash).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot.LLM/Service/AI/LLM/GeminiService.cs` around lines 375 - 404, AnalyzeImageAsync currently falls back to an OpenAI model name ("gpt-4-vision-preview") when modelName is empty; update the fallback to a valid Gemini model identifier (for example "gemini-1.5-flash" or another approved Gemini model) so the CreateGenerativeModel call uses a correct model string. Modify the default assignment of modelName in AnalyzeImageAsync to the chosen Gemini model and ensure the rest of the method (GoogleAi.CreateGenerativeModel("models/" + modelName)) continues to work with that value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs`:
- Around line 27-57: The ExecuteAsync method leaks SkiaSharp resources and can
NRE on bad input: ensure SKBitmap returned by SKBitmap.Decode is checked for
null before calling Encode, and dispose both the SKBitmap (tg_img) and the
SKData (tg_img_data) by using using-blocks (or explicit Dispose) around the
decode/encode operations; keep the existing temp file creation/cleanup and
logging, but move Encode and SaveTo inside the using blocks so tg_img_data is
disposed after writing and tg_img is disposed after encoding.
---
Duplicate comments:
In `@TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs`:
- Line 39: The call to _generalLLMService.AnalyzeImageAsync in LLMOCRService
(using ChatId = 0) bypasses per-chat routing; instead thread the real chat id
from AutoOCRController (use e.Message.Chat.Id) into IOCRService.ExecuteAsync and
propagate it to IGeneralLLMService.AnalyzeImageAsync so LLMOCRService calls
AnalyzeImageAsync(tempPath, chatId, GeneralLLMService.DefaultVisionOcrPrompt,
...) rather than passing 0; update method signatures (IOCRService.ExecuteAsync
and LLMOCRService.ExecuteAsync or related methods) to accept a chatId parameter
and pass it through to _generalLLMService.AnalyzeImageAsync.
- Line 31: The code creates a zero-byte .tmp via Path.GetTempFileName() then
appends ".jpg" to a different filename (tempPath), leaking the original .tmp;
change to create a single temp JPG path and ensure it is deleted after use.
Replace Path.GetTempFileName() + ".jpg" with a proper temp JPG like
Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N") + ".jpg") (or
create the temp file and File.Move to change extension), and add a finally block
or using pattern in LLMOCRService to File.Delete(tempPath) (and also delete the
original .tmp if you choose File.Move) so no temp files are left behind.
---
Nitpick comments:
In `@TelegramSearchBot.LLM/Service/AI/LLM/GeminiService.cs`:
- Around line 375-404: AnalyzeImageAsync currently falls back to an OpenAI model
name ("gpt-4-vision-preview") when modelName is empty; update the fallback to a
valid Gemini model identifier (for example "gemini-1.5-flash" or another
approved Gemini model) so the CreateGenerativeModel call uses a correct model
string. Modify the default assignment of modelName in AnalyzeImageAsync to the
chosen Gemini model and ensure the rest of the method
(GoogleAi.CreateGenerativeModel("models/" + modelName)) continues to work with
that value.
In `@TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs`:
- Around line 1238-1286: In AnalyzeImageAsync the SKBitmap instance tg_img and
the SKData tg_img_data are not disposed on the success path causing a resource
leak; wrap tg_img and tg_img_data in using (or using declarations) so both
SKBitmap (tg_img) and SKData (tg_img_data) are disposed after Encode/ToArray are
called (e.g., using var tg_img = SKBitmap.Decode(fileStream); using var
tg_img_data = tg_img.Encode(...)) while keeping fileStream already in its using,
and ensure BinaryData.FromBytes still receives the byte[] before disposal.
In `@TelegramSearchBot.Test/Service/AI/OCR/LLMOCRServiceTests.cs`:
- Around line 14-41: The test currently doesn't verify the temp .jpg created by
LLMOCRService.ExecuteAsync is deleted; update the mock for
IGeneralLLMService.AnalyzeImageAsync to capture the path argument (e.g., via
Callback or a local variable) when called with
GeneralLLMService.DefaultVisionOcrPrompt, then after awaiting
service.ExecuteAsync(stream) assert that File.Exists(capturedPath) is false to
ensure the temp file was cleaned up; keep the existing setup/verify for
AnalyzeImageAsync and use the capturedPath reference to perform the deletion
assertion.
🪄 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: ecbb122d-eab1-4c43-9f8e-c20b4dfa487a
📒 Files selected for processing (9)
TelegramSearchBot.LLM.Test/Service/AI/LLM/GeneralLLMServiceTests.csTelegramSearchBot.LLM/Interface/AI/LLM/IGeneralLLMService.csTelegramSearchBot.LLM/Interface/AI/LLM/ILLMService.csTelegramSearchBot.LLM/Service/AI/LLM/GeminiService.csTelegramSearchBot.LLM/Service/AI/LLM/GeneralLLMService.csTelegramSearchBot.LLM/Service/AI/LLM/OllamaService.csTelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.csTelegramSearchBot.Test/Service/AI/OCR/LLMOCRServiceTests.csTelegramSearchBot/Service/AI/OCR/LLMOCRService.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- TelegramSearchBot.LLM/Service/AI/LLM/OllamaService.cs
Comment on lines
+27
to
+57
| public async Task<string> ExecuteAsync(Stream file) { | ||
| try { | ||
| var tg_img = SKBitmap.Decode(file); | ||
| var tg_img_data = tg_img.Encode(SKEncodedImageFormat.Jpeg, 99); | ||
| var tempPath = Path.GetTempFileName() + ".jpg"; | ||
|
|
||
| try { | ||
| using (var fs = new FileStream(tempPath, FileMode.Create, FileAccess.Write)) { | ||
| tg_img_data.SaveTo(fs); | ||
| } | ||
|
|
||
| _logger.LogInformation("正在使用LLM进行OCR识别..."); | ||
| var result = await _generalLLMService.AnalyzeImageAsync(tempPath, 0, GeneralLLMService.DefaultVisionOcrPrompt); | ||
|
|
||
| if (string.IsNullOrWhiteSpace(result)) { | ||
| _logger.LogWarning("LLM OCR返回空结果"); | ||
| return string.Empty; | ||
| } | ||
|
|
||
| _logger.LogInformation("LLM OCR识别完成"); | ||
| return result; | ||
| } finally { | ||
| if (File.Exists(tempPath)) { | ||
| File.Delete(tempPath); | ||
| } | ||
| } | ||
| } catch (Exception ex) { | ||
| _logger.LogError(ex, "LLM OCR处理失败"); | ||
| throw; | ||
| } | ||
| } |
There was a problem hiding this comment.
Dispose SkiaSharp objects.
SKBitmap.Decode and SKData returned by Encode are IDisposable. On every OCR request they're leaked until GC finalizes them, which is especially painful for large images. Also, SKBitmap.Decode returns null on undecodable input — a subsequent .Encode will NRE instead of producing a clean error.
- public async Task<string> ExecuteAsync(Stream file) {
- try {
- var tg_img = SKBitmap.Decode(file);
- var tg_img_data = tg_img.Encode(SKEncodedImageFormat.Jpeg, 99);
- var tempPath = Path.GetTempFileName() + ".jpg";
+ public async Task<string> ExecuteAsync(Stream file) {
+ try {
+ using var tg_img = SKBitmap.Decode(file);
+ if (tg_img == null) {
+ _logger.LogWarning("LLM OCR: 无法解码输入图片");
+ return string.Empty;
+ }
+ using var tg_img_data = tg_img.Encode(SKEncodedImageFormat.Jpeg, 99);
+ var tempPath = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.jpg");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/Service/AI/OCR/LLMOCRService.cs` around lines 27 - 57, The
ExecuteAsync method leaks SkiaSharp resources and can NRE on bad input: ensure
SKBitmap returned by SKBitmap.Decode is checked for null before calling Encode,
and dispose both the SKBitmap (tg_img) and the SKData (tg_img_data) by using
using-blocks (or explicit Dispose) around the decode/encode operations; keep the
existing temp file creation/cleanup and logging, but move Encode and SaveTo
inside the using blocks so tg_img_data is disposed after writing and tg_img is
disposed after encoding.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 (1)
TelegramSearchBot.LLM/Service/AI/LLM/GeminiService.cs (1)
461-471:⚠️ Potential issue | 🟠 MajorWrong default model and incorrect Gateway requirement for Gemini.
Two issues in
AnalyzeImageAsync:
- Line 463: the default
modelNamefalls back to"gpt-4-vision-preview"— that's an OpenAI model, not a Gemini one. When a caller doesn't pass a model, the subsequentgoogleAI.CreateGenerativeModel("models/gpt-4-vision-preview")call at line 474 will fail. The default should be a Gemini vision model (e.g."gemini-1.5-flash"or"gemini-2.0-flash", matching the default used inExecAsyncat line 356).- Line 468: requires
channel.Gatewayto be non-empty and returns an error if missing. Gemini is accessed viaGoogleAi(apiKey, ...)and no other method in this file requiresGateway(seeExecAsync,GetAllModels,GenerateEmbeddingsAsync). This check will spuriously reject valid Gemini channels that only have anApiKey.The "Error analyzing image with OpenAI" log message at line 487 also looks copy-pasted from
OpenAIServiceand should say Gemini.🐛 Proposed fix
- public async Task<string> AnalyzeImageAsync(string photoPath, string modelName, LLMChannel channel, string prompt = null) { - if (string.IsNullOrWhiteSpace(modelName)) { - modelName = "gpt-4-vision-preview"; - } - - prompt = string.IsNullOrWhiteSpace(prompt) ? GeneralLLMService.DefaultAltPhotoPrompt : prompt; - - if (channel == null || string.IsNullOrWhiteSpace(channel.Gateway) || string.IsNullOrWhiteSpace(channel.ApiKey)) { - _logger.LogError("{ServiceName}: Channel, Gateway or ApiKey is not configured.", ServiceName); - return $"Error: {ServiceName} channel/gateway/apikey is not configured."; - } + public async Task<string> AnalyzeImageAsync(string photoPath, string modelName, LLMChannel channel, string prompt = null) { + if (string.IsNullOrWhiteSpace(modelName)) { + modelName = "gemini-1.5-flash"; + } + + prompt = string.IsNullOrWhiteSpace(prompt) ? GeneralLLMService.DefaultAltPhotoPrompt : prompt; + + if (channel == null || string.IsNullOrWhiteSpace(channel.ApiKey)) { + _logger.LogError("{ServiceName}: Channel or ApiKey is not configured.", ServiceName); + return $"Error: {ServiceName} channel/apikey is not configured."; + } @@ - } catch (Exception ex) { - _logger.LogError(ex, "Error analyzing image with OpenAI"); + } catch (Exception ex) { + _logger.LogError(ex, "Error analyzing image with Gemini"); return $"Error analyzing image: {ex.Message}"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot.LLM/Service/AI/LLM/GeminiService.cs` around lines 461 - 471, AnalyzeImageAsync currently defaults modelName to an OpenAI model and incorrectly requires channel.Gateway; change the default modelName to the Gemini vision default used by ExecAsync (e.g., "gemini-1.5-flash" or "gemini-2.0-flash") so the subsequent googleAI.CreateGenerativeModel(...) call succeeds, remove the check for channel.Gateway so only channel.ApiKey is required (match other Gemini usages like ExecAsync/GetAllModels/GenerateEmbeddingsAsync), and update the error log message from "Error analyzing image with OpenAI" to reference Gemini (e.g., "Error analyzing image with Gemini") to avoid the OpenAI copy-paste text.
♻️ Duplicate comments (2)
TelegramSearchBot/Controller/Manage/EditOCRConfController.cs (1)
21-21:⚠️ Potential issue | 🔴 CriticalDo not reuse a mutable view instance across updates.
EditOCRConfViewstores_chatId,_replyToMessageId,_messageText, and_replyMarkup; mutating the injected field per request can cross-contaminate concurrent admin updates. Make the view render method accept these values as parameters, or resolve a fresh view per execution.As per coding guidelines, controllers must “remain stateless with all persistence delegated to
Service/*classes”.Also applies to: 53-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Controller/Manage/EditOCRConfController.cs` at line 21, EditOCRConfController currently reuses a mutable EditOCRConfView instance (field EditOCRConfView) which stores per-request fields like _chatId, _replyToMessageId, _messageText and _replyMarkup and can cause cross-request contamination; update the controller to stop mutating/injecting a single view instance by either (A) changing EditOCRConfView to expose a render/show method that accepts chatId, replyToMessageId, messageText and replyMarkup as parameters and remove per-request state from the view, or (B) resolve/create a fresh EditOCRConfView per request inside the controller action so the controller remains stateless; update any calls in EditOCRConfController that set those fields to pass parameters into the new render method or to new instances accordingly and ensure persistence stays in Service/* classes.TelegramSearchBot/Service/Manage/EditOCRConfService.cs (1)
77-81:⚠️ Potential issue | 🟡 MinorRecover from unknown Redis states instead of silently ignoring the command.
Line 81 still returns
(false, string.Empty)for unrecognized state values, which can dead-end the OCR configuration conversation. Reset toMainMenuand return the main menu prompt.🛠️ Proposed recovery
if (_stateHandlers.TryGetValue(currentState, out var handler)) { return await handler(redis, normalizedCommand); } - return (false, string.Empty); + await redis.SetStateAsync(OCRConfState.MainMenu.GetDescription()); + return (true, await BuildMainMenuMessageAsync());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/Manage/EditOCRConfService.cs` around lines 77 - 81, When no handler is found in _stateHandlers for currentState, don't return (false, string.Empty); instead reset the conversation state in redis to MainMenu and return the main menu prompt so the user can continue; locate the failure branch where _stateHandlers.TryGetValue(currentState, out var handler) fails and replace the fallback with a call on redis to set the state to MainMenu (e.g., redis.SetStateAsync or the project's equivalent) and return the main menu prompt string (rather than an empty string).
🧹 Nitpick comments (2)
TelegramSearchBot/TelegramSearchBot.csproj (1)
106-110: Remove the duplicatetelegram-bot-api.execontent item.Lines 106-110 repeat the same
Content Include="telegram-bot-api.exe"rule already present on Lines 101-105. Keep a single item to avoid redundant copy metadata.Proposed cleanup
- <ItemGroup> - <Content Include="telegram-bot-api.exe" Condition="Exists('telegram-bot-api.exe')"> - <CopyToOutputDirectory>Always</CopyToOutputDirectory> - </Content> - </ItemGroup>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/TelegramSearchBot.csproj` around lines 106 - 110, The project file contains a duplicated Content entry for "telegram-bot-api.exe" causing redundant copy-to-output metadata; remove the second duplicate Content Include="telegram-bot-api.exe" block (the repeated ItemGroup lines 106-110) so only one Content Include="telegram-bot-api.exe" with <CopyToOutputDirectory>Always</CopyToOutputDirectory> remains in the .csproj, leaving the original Content/ItemGroup and its metadata intact.TelegramSearchBot.Test/Manage/EditOCRConfServiceTests.cs (1)
104-109: Assert the state transition instead of overwriting it.Line 108 masks the behavior this test name claims to verify. If
ExecuteAsync("切换OCR引擎", 100)stops settingSelectingEngine, this test would still pass.🧪 Proposed test tightening
var (switched, engineMessage) = await _service.ExecuteAsync("切换OCR引擎", 100); Assert.True(switched); Assert.Contains("请选择 OCR 引擎", engineMessage); + Assert.True(_redisStore.TryGetValue("ocrconf:100:state", out var state)); + Assert.Equal(OCRConfState.SelectingEngine.GetDescription(), state.ToString()); - _redisStore["ocrconf:100:state"] = OCRConfState.SelectingEngine.GetDescription(); var engineMarkup = await _service.GetReplyMarkupAsync(100);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot.Test/Manage/EditOCRConfServiceTests.cs` around lines 104 - 109, The test currently overwrites the state after calling _service.ExecuteAsync("切换OCR引擎", 100); instead of asserting the state transition; remove the manual assignment to _redisStore["ocrconf:100:state"] and assert that ExecuteAsync set the state to OCRConfState.SelectingEngine.GetDescription() (or equivalent) before calling _service.GetReplyMarkupAsync(100), so the test verifies ExecuteAsync actually transitions the state to SelectingEngine.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@TelegramSearchBot.LLM/Service/AI/LLM/GeneralLLMService.cs`:
- Around line 262-266: The AnalyzeImageAsync method in GeneralLLMService is
dropping the provided CancellationToken when calling the provider; update the
call to forward the token to the provider's cancellation-aware overload (i.e.,
call ILLMService.AnalyzeImageAsync(PhotoPath, modelName, channel, prompt,
cancellationToken)) and, if that overload doesn't exist on ILLMService, add a
cancellation-aware overload there and implement it in concrete providers so
long-running image analysis can be cancelled; ensure the method still yields the
provider result and respects the token for cooperative cancellation.
In `@TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs`:
- Around line 1464-1469: Replace the retired hardcoded model in
AnalyzeImageAsync by defaulting modelName to Env.OpenAIModelName when
null/whitespace (update the logic in AnalyzeImageAsync), and change the
class-level attribute from [Injectable(ServiceLifetime.Transient)] to
[Injectable(ServiceLifetime.Singleton)] on the OpenAIService class so the
service is registered as a singleton; update any related constructor-injected
dependencies if necessary to be singleton-safe.
In `@TelegramSearchBot/AppBootstrap/GeneralBootstrap.cs`:
- Around line 115-130: The PR introduces a duplicate startup path for the
embedded telegram-bot-api and a redundant readiness helper: remove the extra
launch and use the existing utilities instead — stop calling or starting a
second instance in the new branch that invokes StartEmbeddedLocalBotApiAsync
when Env.EnableLocalBotAPI is true, delete or collapse the new
WaitForLocalBotApiReady helper and replace its usages with the existing
WaitForTcpServiceReady call using Env.LocalBotApiPort, and ensure all readiness
checks reference WaitForTcpServiceReady so we never attempt to start the
executable twice.
In `@TelegramSearchBot/Service/Manage/EditOCRConfService.cs`:
- Line 21: The service EditOCRConfService is decorated with
[Injectable(ServiceLifetime.Transient)] but project convention requires
singleton registration; change the attribute on EditOCRConfService to
[Injectable(ServiceLifetime.Singleton)] and resolve DbContext lifetime issues by
injecting a context factory instead of a scoped DataDbContext directly — e.g.,
replace any constructor parameters of DataDbContext with an
IDbContextFactory<DataDbContext> (or a factory/interface you already use) and
update methods in EditOCRConfService to create contexts from that factory; keep
the class name EditOCRConfService and the Injectable attribute as the points of
change so Scrutor DI scanning will register it as Singleton.
- Around line 102-105: Defer calling SetEngineAsync(OCREngine.LLM) until after a
valid LLM channel and model are chosen: change the flow in the command handling
(the block that now calls SetEngineAsync then
redis.SetStateAsync(OCRConfState.SelectingLLMChannel.GetDescription()) and
returns BuildLLMChannelSelectionMessageAsync()) so it only sets Redis to
SelectingLLMChannel and does NOT call SetEngineAsync yet; perform
SetEngineAsync(OCREngine.LLM) only after a successful model selection in the
handler that currently accepts typed models (the SelectingLLMModel handling
around line ~133) and after validating the typed model against the available
configured models (if any exist, reject/reprompt invalid entries); also move the
no-channel fallback logic (currently around 282–287) into a single handler that
resets Redis state to MainMenu and ensures the engine is not switched (call
SetEngineAsync(OCREngine.PaddleOCR) or leave unchanged) so
exiting/typos/no-channels cannot leave the system in an incomplete LLM state;
apply the same deferred-engine and validation changes to the matching code paths
noted (126–136 and 282–287).
In `@TelegramSearchBot/View/EditOCRConfView.cs`:
- Line 10: Add the Scrutor registration attribute to this view and make its
render behavior stateless: annotate the EditOCRConfView class with
[Injectable(ServiceLifetime.Singleton)] so Scrutor will register it as a
singleton (matching the EditOCRConfController constructor-injection), and
refactor any instance state used by its Render/RenderPath methods
(implementations of IView) to be local or passed in as parameters so the
singleton can be safely shared across requests.
---
Outside diff comments:
In `@TelegramSearchBot.LLM/Service/AI/LLM/GeminiService.cs`:
- Around line 461-471: AnalyzeImageAsync currently defaults modelName to an
OpenAI model and incorrectly requires channel.Gateway; change the default
modelName to the Gemini vision default used by ExecAsync (e.g.,
"gemini-1.5-flash" or "gemini-2.0-flash") so the subsequent
googleAI.CreateGenerativeModel(...) call succeeds, remove the check for
channel.Gateway so only channel.ApiKey is required (match other Gemini usages
like ExecAsync/GetAllModels/GenerateEmbeddingsAsync), and update the error log
message from "Error analyzing image with OpenAI" to reference Gemini (e.g.,
"Error analyzing image with Gemini") to avoid the OpenAI copy-paste text.
---
Duplicate comments:
In `@TelegramSearchBot/Controller/Manage/EditOCRConfController.cs`:
- Line 21: EditOCRConfController currently reuses a mutable EditOCRConfView
instance (field EditOCRConfView) which stores per-request fields like _chatId,
_replyToMessageId, _messageText and _replyMarkup and can cause cross-request
contamination; update the controller to stop mutating/injecting a single view
instance by either (A) changing EditOCRConfView to expose a render/show method
that accepts chatId, replyToMessageId, messageText and replyMarkup as parameters
and remove per-request state from the view, or (B) resolve/create a fresh
EditOCRConfView per request inside the controller action so the controller
remains stateless; update any calls in EditOCRConfController that set those
fields to pass parameters into the new render method or to new instances
accordingly and ensure persistence stays in Service/* classes.
In `@TelegramSearchBot/Service/Manage/EditOCRConfService.cs`:
- Around line 77-81: When no handler is found in _stateHandlers for
currentState, don't return (false, string.Empty); instead reset the conversation
state in redis to MainMenu and return the main menu prompt so the user can
continue; locate the failure branch where
_stateHandlers.TryGetValue(currentState, out var handler) fails and replace the
fallback with a call on redis to set the state to MainMenu (e.g.,
redis.SetStateAsync or the project's equivalent) and return the main menu prompt
string (rather than an empty string).
---
Nitpick comments:
In `@TelegramSearchBot.Test/Manage/EditOCRConfServiceTests.cs`:
- Around line 104-109: The test currently overwrites the state after calling
_service.ExecuteAsync("切换OCR引擎", 100); instead of asserting the state
transition; remove the manual assignment to _redisStore["ocrconf:100:state"] and
assert that ExecuteAsync set the state to
OCRConfState.SelectingEngine.GetDescription() (or equivalent) before calling
_service.GetReplyMarkupAsync(100), so the test verifies ExecuteAsync actually
transitions the state to SelectingEngine.
In `@TelegramSearchBot/TelegramSearchBot.csproj`:
- Around line 106-110: The project file contains a duplicated Content entry for
"telegram-bot-api.exe" causing redundant copy-to-output metadata; remove the
second duplicate Content Include="telegram-bot-api.exe" block (the repeated
ItemGroup lines 106-110) so only one Content Include="telegram-bot-api.exe" with
<CopyToOutputDirectory>Always</CopyToOutputDirectory> remains in the .csproj,
leaving the original Content/ItemGroup and its metadata intact.
🪄 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: 073638f5-e6b2-4b20-81bb-d918ab40f470
📒 Files selected for processing (12)
TelegramSearchBot.LLM.Test/Service/AI/LLM/GeneralLLMServiceTests.csTelegramSearchBot.LLM/Service/AI/LLM/AnthropicService.csTelegramSearchBot.LLM/Service/AI/LLM/GeminiService.csTelegramSearchBot.LLM/Service/AI/LLM/GeneralLLMService.csTelegramSearchBot.LLM/Service/AI/LLM/OllamaService.csTelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.csTelegramSearchBot.Test/Manage/EditOCRConfServiceTests.csTelegramSearchBot/AppBootstrap/GeneralBootstrap.csTelegramSearchBot/Controller/Manage/EditOCRConfController.csTelegramSearchBot/Service/Manage/EditOCRConfService.csTelegramSearchBot/TelegramSearchBot.csprojTelegramSearchBot/View/EditOCRConfView.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- TelegramSearchBot.LLM.Test/Service/AI/LLM/GeneralLLMServiceTests.cs
Comment on lines
+262
to
266
| public async IAsyncEnumerable<string> AnalyzeImageAsync(string PhotoPath, long ChatId, string modelName, ILLMService service, LLMChannel channel, string prompt, CancellationToken cancellationToken = default) { | ||
| prompt = string.IsNullOrWhiteSpace(prompt) ? DefaultAltPhotoPrompt : prompt; | ||
| yield return await service.AnalyzeImageAsync(PhotoPath, modelName, channel, prompt); | ||
| yield break; | ||
| } |
There was a problem hiding this comment.
Cancellation token is dropped when calling the provider.
The new streaming overload accepts cancellationToken but doesn't forward it — service.AnalyzeImageAsync(PhotoPath, modelName, channel, prompt) is awaited without cancellation support. If ILLMService.AnalyzeImageAsync has a cancellation-aware overload, pass the token through; otherwise consider adding one, since image analysis can be a long HTTP call and the rest of this service plumbs CancellationToken consistently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot.LLM/Service/AI/LLM/GeneralLLMService.cs` around lines 262 -
266, The AnalyzeImageAsync method in GeneralLLMService is dropping the provided
CancellationToken when calling the provider; update the call to forward the
token to the provider's cancellation-aware overload (i.e., call
ILLMService.AnalyzeImageAsync(PhotoPath, modelName, channel, prompt,
cancellationToken)) and, if that overload doesn't exist on ILLMService, add a
cancellation-aware overload there and implement it in concrete providers so
long-running image analysis can be cancelled; ensure the method still yields the
provider result and respects the token for cooperative cancellation.
Comment on lines
+1464
to
+1469
| public async Task<string> AnalyzeImageAsync(string photoPath, string modelName, LLMChannel channel, string prompt = null) { | ||
| if (string.IsNullOrWhiteSpace(modelName)) { | ||
| modelName = "gpt-4-vision-preview"; | ||
| } | ||
|
|
||
| prompt = string.IsNullOrWhiteSpace(prompt) ? GeneralLLMService.DefaultAltPhotoPrompt : prompt; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is OpenAI gpt-4-vision-preview still available in 2026 or has it been retired?
💡 Result:
No, OpenAI's gpt-4-vision-preview model has been retired and is no longer available as of 2026. It was deprecated with a shutdown date of December 6, 2024, and is not listed in current OpenAI API models or deprecations for 2026. Modern models like gpt-4o, gpt-5.4 series support vision capabilities.
Citations:
- 1: https://platform.openai.com/docs/deprecations/
- 2: https://developers.openai.com/api/docs/deprecations
- 3: https://developers.openai.com/api/docs/models/
- 4: https://platform.openai.com/docs/guides/vision
- 5: https://platform.openai.com/docs/deprecations
🏁 Script executed:
# Search for the OpenAIService class definition and interface implementation
fd -e cs "OpenAIService" | head -20Repository: ModerRAS/TelegramSearchBot
Length of output: 123
🏁 Script executed:
# Check if the class implements ILLMService
rg "class OpenAIService" -A 5 --type csRepository: ModerRAS/TelegramSearchBot
Length of output: 615
🏁 Script executed:
# Look for MaxToolCycles in the codebase
rg "MaxToolCycles" --type cs -B 2 -A 2Repository: ModerRAS/TelegramSearchBot
Length of output: 11785
🏁 Script executed:
# Check how Env.OpenAIModelName is used for configuration pattern
rg "Env\.OpenAI" --type cs -B 2 -A 2 | head -40Repository: ModerRAS/TelegramSearchBot
Length of output: 1301
🏁 Script executed:
# Look for Injectable attribute usage in the codebase
rg "Injectable.*Singleton" --type cs -B 1 -A 1 | head -30Repository: ModerRAS/TelegramSearchBot
Length of output: 2774
🏁 Script executed:
# Get the full class definition with attributes
rg -B 5 "public class OpenAIService" --type csRepository: ModerRAS/TelegramSearchBot
Length of output: 641
🏁 Script executed:
# Search for vision model or image analysis related configuration in Env
rg "Vision|Image|Photo" TelegramSearchBot.Common/Env.cs --type cs -B 1 -A 1Repository: ModerRAS/TelegramSearchBot
Length of output: 52
🏁 Script executed:
sed -n '1464,1475p' TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs | cat -nRepository: ModerRAS/TelegramSearchBot
Length of output: 858
Fix the deprecated vision model and correct the service lifetime registration.
The hardcoded "gpt-4-vision-preview" model has been retired by OpenAI (shutdown: December 6, 2024) and is no longer available. Replace it with the configuration pattern used elsewhere in the class: default to Env.OpenAIModelName when modelName is not supplied. Additionally, change the class-level [Injectable(ServiceLifetime.Transient)] to [Injectable(ServiceLifetime.Singleton)] to comply with the LLM service guideline for TelegramSearchBot.LLM/**/Service/AI/LLM/**/*.cs files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs` around lines 1464 -
1469, Replace the retired hardcoded model in AnalyzeImageAsync by defaulting
modelName to Env.OpenAIModelName when null/whitespace (update the logic in
AnalyzeImageAsync), and change the class-level attribute from
[Injectable(ServiceLifetime.Transient)] to
[Injectable(ServiceLifetime.Singleton)] on the OpenAIService class so the
service is registered as a singleton; update any related constructor-injected
dependencies if necessary to be singleton-safe.
Comment on lines
+115
to
+130
| /// <summary> | ||
| /// 等待本地 telegram-bot-api 服务端口就绪，最多等待 20 秒 | ||
| /// </summary> | ||
| private static async Task WaitForLocalBotApiReady(int port, int maxRetries = 40, int delayMs = 500) { | ||
| for (int i = 0; i < maxRetries; i++) { | ||
| try { | ||
| using var tcp = new System.Net.Sockets.TcpClient(); | ||
| await tcp.ConnectAsync("127.0.0.1", port); | ||
| Log.Information("telegram-bot-api 服务已就绪 (端口 {Port})，耗时约 {ElapsedMs}ms", port, i * delayMs); | ||
| return; | ||
| } catch { | ||
| await Task.Delay(delayMs); | ||
| } | ||
| } | ||
| Log.Warning("等待 telegram-bot-api 服务就绪超时 (端口 {Port})，将继续启动", port); | ||
| } |
There was a problem hiding this comment.
Avoid starting telegram-bot-api twice.
Startup already calls StartEmbeddedLocalBotApiAsync() on Lines 153-154 when Env.EnableLocalBotAPI is true. This added branch starts the same executable again on the same Env.LocalBotApiPort, so the second process can fail to bind or make readiness checks pass against the first process. The new WaitForLocalBotApiReady helper is also redundant with WaitForTcpServiceReady.
Proposed cleanup
- /// <summary>
- /// 等待本地 telegram-bot-api 服务端口就绪，最多等待 20 秒
- /// </summary>
- private static async Task WaitForLocalBotApiReady(int port, int maxRetries = 40, int delayMs = 500) {
- for (int i = 0; i < maxRetries; i++) {
- try {
- using var tcp = new System.Net.Sockets.TcpClient();
- await tcp.ConnectAsync("127.0.0.1", port);
- Log.Information("telegram-bot-api 服务已就绪 (端口 {Port})，耗时约 {ElapsedMs}ms", port, i * delayMs);
- return;
- } catch {
- await Task.Delay(delayMs);
- }
- }
- Log.Warning("等待 telegram-bot-api 服务就绪超时 (端口 {Port})，将继续启动", port);
- }
-
...
- // 如果启用了本地 telegram-bot-api，则在此启动它
- if (Env.EnableLocalBotAPI) {
- string botApiExePath = Path.Combine(AppContext.BaseDirectory, "telegram-bot-api.exe");
- if (File.Exists(botApiExePath)) {
- if (string.IsNullOrEmpty(Env.TelegramBotApiId) || string.IsNullOrEmpty(Env.TelegramBotApiHash)) {
- Log.Warning("EnableLocalBotAPI 为 true，但 TelegramBotApiId 或 TelegramBotApiHash 未配置，跳过本地 Bot API 启动");
- } else {
- var botApiDataDir = Path.Combine(Env.WorkDir, "telegram-bot-api");
- Directory.CreateDirectory(botApiDataDir);
- // 使用 ArgumentList 以正确处理路径中的空格
- // --local 模式允许大文件上传下载并将文件存储在本地 dir 下
- var startInfo = new ProcessStartInfo {
- FileName = botApiExePath,
- UseShellExecute = false,
- RedirectStandardOutput = true,
- RedirectStandardError = true
- };
- startInfo.ArgumentList.Add("--local");
- startInfo.ArgumentList.Add($"--api-id={Env.TelegramBotApiId}");
- startInfo.ArgumentList.Add($"--api-hash={Env.TelegramBotApiHash}");
- startInfo.ArgumentList.Add($"--dir={botApiDataDir}");
- startInfo.ArgumentList.Add($"--http-port={Env.LocalBotApiPort}");
- var botApiProcess = Process.Start(startInfo);
- if (botApiProcess == null) {
- Log.Warning("telegram-bot-api 进程启动失败");
- } else {
- childProcessManager.AddProcess(botApiProcess);
- Log.Information("telegram-bot-api 已启动，等待端口 {Port} 就绪...", Env.LocalBotApiPort);
- await WaitForLocalBotApiReady(Env.LocalBotApiPort);
- }
- }
- } else {
- Log.Warning("未找到 telegram-bot-api 可执行文件 {Path}，跳过本地 Bot API 启动", botApiExePath);
- }
- }
-Also applies to: 159-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/AppBootstrap/GeneralBootstrap.cs` around lines 115 - 130,
The PR introduces a duplicate startup path for the embedded telegram-bot-api and
a redundant readiness helper: remove the extra launch and use the existing
utilities instead — stop calling or starting a second instance in the new branch
that invokes StartEmbeddedLocalBotApiAsync when Env.EnableLocalBotAPI is true,
delete or collapse the new WaitForLocalBotApiReady helper and replace its usages
with the existing WaitForTcpServiceReady call using Env.LocalBotApiPort, and
ensure all readiness checks reference WaitForTcpServiceReady so we never attempt
to start the executable twice.
| using TelegramSearchBot.Model.Data; | ||
|
|
||
| namespace TelegramSearchBot.Service.Manage { | ||
| [Injectable(Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient)] |
There was a problem hiding this comment.
Align the service lifetime with the repository DI convention.
This service is registered as Transient, while the project convention requires [Injectable(ServiceLifetime.Singleton)]. If DataDbContext lifetime prevents singleton registration, use a context factory rather than changing the service lifetime ad hoc.
As per coding guidelines, **/*.cs files under TelegramSearchBot should “Use Scrutor DI scanning with [Injectable(ServiceLifetime.Singleton)] attribute for service registration in namespaces under TelegramSearchBot”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/Service/Manage/EditOCRConfService.cs` at line 21, The
service EditOCRConfService is decorated with
[Injectable(ServiceLifetime.Transient)] but project convention requires
singleton registration; change the attribute on EditOCRConfService to
[Injectable(ServiceLifetime.Singleton)] and resolve DbContext lifetime issues by
injecting a context factory instead of a scoped DataDbContext directly — e.g.,
replace any constructor parameters of DataDbContext with an
IDbContextFactory<DataDbContext> (or a factory/interface you already use) and
update methods in EditOCRConfService to create contexts from that factory; keep
the class name EditOCRConfService and the Injectable attribute as the points of
change so Scrutor DI scanning will register it as Singleton.
Comment on lines
+102
to
+105
| } else if (command == "2" || command == "LLM") { | ||
| await SetEngineAsync(OCREngine.LLM); | ||
| await redis.SetStateAsync(OCRConfState.SelectingLLMChannel.GetDescription()); | ||
| return (true, await BuildLLMChannelSelectionMessageAsync()); |
There was a problem hiding this comment.
Make LLM engine activation atomic with valid channel/model selection.
Line 103 switches the global OCR engine to LLM before channel/model selection completes, Line 133 accepts any text as a model, and Line 285 can switch back to PaddleOCR without resetting Redis state. Exiting, typoing a model, or having no channels can leave AutoOCR using an incomplete/invalid LLM config or showing the wrong keyboard.
Defer SetEngineAsync(OCREngine.LLM) until after a valid channel/model is selected, validate typed models when configured models exist, and move the no-channel fallback into a handler that can also set Redis state back to MainMenu.
🛠️ Directional fix
} else if (command == "2" || command == "LLM") {
- await SetEngineAsync(OCREngine.LLM);
+ var channels = await GetAvailableLLMChannelsAsync();
+ if (!channels.Any()) {
+ await SetEngineAsync(OCREngine.PaddleOCR);
+ await redis.SetStateAsync(OCRConfState.MainMenu.GetDescription());
+ return (true, $"当前没有可用的 LLM 渠道，已切回 PaddleOCR。{System.Environment.NewLine}{System.Environment.NewLine}{await BuildMainMenuMessageAsync()}");
+ }
await redis.SetStateAsync(OCRConfState.SelectingLLMChannel.GetDescription());
return (true, await BuildLLMChannelSelectionMessageAsync());
}- await SetLLMConfigAsync(channelId.Value, command);
+ var models = await GetAvailableModelsAsync(channelId.Value);
+ if (models.Any() && !models.Contains(command)) {
+ return (true, $"无效的模型，请使用下方键盘选择。{System.Environment.NewLine}{System.Environment.NewLine}{await BuildLLMModelSelectionMessageAsync(channelId.Value, string.Empty)}");
+ }
+
+ await SetLLMConfigAsync(channelId.Value, command);
+ await SetEngineAsync(OCREngine.LLM);
await redis.SetStateAsync(OCRConfState.MainMenu.GetDescription());Also applies to: 126-136, 282-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/Service/Manage/EditOCRConfService.cs` around lines 102 -
105, Defer calling SetEngineAsync(OCREngine.LLM) until after a valid LLM channel
and model are chosen: change the flow in the command handling (the block that
now calls SetEngineAsync then
redis.SetStateAsync(OCRConfState.SelectingLLMChannel.GetDescription()) and
returns BuildLLMChannelSelectionMessageAsync()) so it only sets Redis to
SelectingLLMChannel and does NOT call SetEngineAsync yet; perform
SetEngineAsync(OCREngine.LLM) only after a successful model selection in the
handler that currently accepts typed models (the SelectingLLMModel handling
around line ~133) and after validating the typed model against the available
configured models (if any exist, reject/reprompt invalid entries); also move the
no-channel fallback logic (currently around 282–287) into a single handler that
resets Redis state to MainMenu and ensures the engine is not switched (call
SetEngineAsync(OCREngine.PaddleOCR) or leave unchanged) so
exiting/typos/no-channels cannot leave the system in an incomplete LLM state;
apply the same deferred-engine and validation changes to the matching code paths
noted (126–136 and 282–287).
| using TelegramSearchBot.Service.BotAPI; | ||
|
|
||
| namespace TelegramSearchBot.View { | ||
| public class EditOCRConfView : IView { |
There was a problem hiding this comment.
Register this DI-resolved view with the project’s injectable convention.
EditOCRConfController constructor-injects EditOCRConfView, but this class has no [Injectable(ServiceLifetime.Singleton)] marker for Scrutor scanning. Add the attribute after making the render path stateless as noted in the controller comment.
As per coding guidelines, **/*.cs files under TelegramSearchBot should “Use Scrutor DI scanning with [Injectable(ServiceLifetime.Singleton)] attribute for service registration in namespaces under TelegramSearchBot”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot/View/EditOCRConfView.cs` at line 10, Add the Scrutor
registration attribute to this view and make its render behavior stateless:
annotate the EditOCRConfView class with [Injectable(ServiceLifetime.Singleton)]
so Scrutor will register it as a singleton (matching the EditOCRConfController
constructor-injection), and refactor any instance state used by its
Render/RenderPath methods (implementations of IView) to be local or passed in as
parameters so the singleton can be safely shared across requests.
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
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.
变更摘要
实现Issue #241 - 添加基于大模型的OCR作为PaddleOCR的替代方案
新增功能
IOCRService接口和OCREngine枚举
LLMOCRService实现
OCR配置状态机
AutoOCRController增强
使用方式
通过Bot私聊发送以下命令进行OCR配置：
文件变更
新增文件：
修改文件：
Summary by CodeRabbit
Release Notes
New Features
Improvements
Tests