New translation engine Ollama #2
Conversation
📝 WalkthroughWalkthroughAdded an Ollama-based translator and renamed/refactored AI translator base types, removed Ready/Prepare lifecycle, introduced concurrent worker threads with optional per-request sleep, added settings for sleep and worker count, adjusted API key rotation/storage, tweaked cache export and JSON helpers, and updated project and IDE document lists. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI/Enqueue
participant Manager as TranslatorManager (Worker Pool)
participant Translator as Translator (e.g., Ollama)
participant API as External API (Ollama/DeepL/Google)
UI->>Manager: Enqueue text for translation
Manager->>Manager: Wait if Settings.SleepTime > 0
Manager->>Translator: Dequeue and request translation (text)
Translator->>API: HTTP request (models or /chat/completions)
API-->>Translator: JSON response
Translator->>Translator: ParseResponse (clean, strip, normalize)
Translator-->>Manager: Translated text
Manager-->>UI: Deliver translated result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Source/AutoTranslation/Translators/Translator_Ollama.cs (3)
13-18: Consider making the base URL configurable.The hardcoded localhost URL may limit flexibility for users who run Ollama on different hosts or ports.
Consider making the BaseURL configurable through settings or allowing it to be overridden:
-public override string BaseURL => "http://localhost:11434/v1/"; +public override string BaseURL => Settings.OllamaBaseURL ?? "http://localhost:11434/v1/";
42-54: Consider using structured JSON construction.While the string interpolation approach works, using a more structured approach could improve maintainability and reduce the risk of malformed JSON.
Consider using a JSON library or structured approach:
-var requestBody = $@"{{ - ""model"": ""{Model}"", - ""messages"": [ - {{ - ""role"": ""{RoleSystem}"", - ""content"": ""{Prompt.EscapeJsonString()}"" - }}, - {{ - ""role"": ""user"", - ""content"": ""{text.EscapeJsonString()}"" - }} - ] -}}"; +var messages = new[] +{ + new { role = RoleSystem, content = Prompt }, + new { role = "user", content = text } +}; +var requestBody = JsonConvert.SerializeObject(new { model = Model, messages = messages });
97-108: Effective response cleaning addresses reported issues.The text cleaning logic properly addresses the unwanted characters and tags mentioned in the PR objectives. The approach systematically removes escaped HTML,
<think>tags, and normalizes whitespace.Consider minor performance optimization by combining string replacements:
-content = content.Replace(@"\u003c", "<").Replace(@"\u003e", ">"); - -content = System.Text.RegularExpressions.Regex.Replace(content, @"<think>.*?</think>", "", - System.Text.RegularExpressions.RegexOptions.Singleline | - System.Text.RegularExpressions.RegexOptions.IgnoreCase); - -content = content.Replace(@"\n", " "); -content = content.Replace(@"\r", " "); - -content = System.Text.RegularExpressions.Regex.Replace(content, @"\s+", " "); +// Unescape HTML entities +content = content.Replace(@"\u003c", "<").Replace(@"\u003e", ">"); + +// Remove think tags and normalize line breaks in one pass +content = System.Text.RegularExpressions.Regex.Replace(content, @"<think>.*?</think>|\\[nr]", " ", + System.Text.RegularExpressions.RegexOptions.Singleline | + System.Text.RegularExpressions.RegexOptions.IgnoreCase); + +// Normalize whitespace +content = System.Text.RegularExpressions.Regex.Replace(content, @"\s+", " ");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Source/AutoTranslation/AutoTranslation.csproj(1 hunks)Source/AutoTranslation/Translators/Translator_Ollama.cs(1 hunks)
🔇 Additional comments (9)
Source/AutoTranslation/AutoTranslation.csproj (3)
107-107: LGTM!The new Translator_Ollama.cs file is correctly added to the compilation list in the appropriate location among other translator files.
107-107: LGTM! Correct integration of new translator.The addition of
Translator_Ollama.csto the compilation list is properly placed among other translator implementations and follows the established project structure.
107-107: Verify thatTranslator_Ollama.csis present at the declared pathThe build will fail if
Source/AutoTranslation/Translators/Translator_Ollama.csis missing, mis-named, or located in a differently-cased folder (especially on CI runners with case-sensitive filesystems). Double-check the commit to ensure the file is included exactly at this relative path.Source/AutoTranslation/Translators/Translator_Ollama.cs (6)
20-38: LGTM! Good error handling implementation.The GetModels() method properly handles the API request with appropriate error handling and user feedback. The Authorization header addition is harmless even if not required by local Ollama instances.
73-92: Good defensive parsing strategy.The multiple fallback attempts to parse JSON responses from different possible formats is a robust approach that handles various API response structures.
13-18: LGTM! Clean class structure following established patterns.The class properly extends the base AI model translator and uses the correct Ollama API endpoint. The virtual
RoleSystemproperty provides good extensibility.
20-38: Verify if Authorization header is needed for Ollama.The method structure and error handling look good. However, Ollama typically runs without authentication by default. The Authorization header on line 26 might be unnecessary and could potentially cause requests to fail.
Consider making the Authorization header conditional or removing it if not needed:
- request.Headers.Add("Authorization", "Bearer " + APIKey); + if (!string.IsNullOrEmpty(APIKey)) + { + request.Headers.Add("Authorization", "Bearer " + APIKey); + }
40-67: Well-structured request method with same auth concern.The JSON request construction and HTTP setup are implemented correctly, following OpenAI API standards. However, the same Authorization header concern from
GetModels()applies here (line 59).The method properly constructs the OpenAI-compatible request, but consider the same Authorization header fix as mentioned in the previous comment.
69-109: Excellent response parsing with comprehensive cleanup logic.This method effectively addresses the issues mentioned in the PR objectives regarding unwanted characters and
<think>tags. The multiple fallback strategies for different response formats show good defensive programming.The response cleaning logic is particularly well-thought-out:
- HTML entity unescaping for proper character display
- Removal of reasoning tags that shouldn't appear in final translations
- Normalization of whitespace and line breaks
This directly addresses the problems mentioned in the PR description about unwanted newline symbols and
<think>tags.
|
Thank you for your contribution. Before I put it on the Steam Workshop, I'd like to ask you to confirm that your additions work, I don't know about Ollama so I need real user feedback. (But I at least checked Ollama's API match with the code you added) You can also send me a Steam friend request, and I'll add you as a contributor. |
|
Hello again. I tried again with the mode you posted, and it seems to be working, but there are a few errors I noticed other than this addition. I haven't debugged them yet, but there seem to be some fundamental issues. The first error is related to the counter status. The translation rate counter for a translated mode seems to be behaving incorrectly. I assume I’m not misinterpreting the numbers. It seems like progress can’t be tracked. 
I think the numbers here are not being reset after the translation reset.
The second error occurs in some models. An easy way to test this is to use the Gemini API to translate with the Gemma3 27B model (a model with free API access). The translation requests are going through correctly, and even the test translation is working properly, but the translations are not being saved to the cache even though they are being deducted from the counter. This also happened with some Ollama models I tested. In addition, some thinking models can cause package requests to time out because they extend the response time too much. Perhaps we could consider extending the request time. |
|
An error occurs when using Translator_Ollama: <color=#34e2eb>AutoTranslation: Selected translator named Ollama is not ready, changing to other translator.. Google This happens because Translator_Ollama, which inherits from Translator_BaseOnlineAIModel, does not set the Ready variable to true in the Prepare() method due to the API key check logic. This behavior needs to be fixed. Additionally, based on the internal logic, inheriting from Translator_BaseOnlineAIModel might not be the right approach. Translator_BaseOnlineAIModel is intended for AI models that require an API key, whereas Ollama is designed to work locally without one. Would it be better to introduce a new class like Translator_BaseOfflineAIModel instead? A more structured class hierarchy could help support not only Ollama but also other offline models in the future. |
|
I think By deleting Also, like @StartaBafras said, I think we should consider adding a request time offset, which defaults to 0 seconds as it is now, but users could increase it to X second on their own to avoid API rate limits, etc. I'll develop those features when I'm done with work today. |
|
I didn't notice the error because the API key remained written while constantly switching between Gemini and Ollama. Thank you very much. @PMK95 If we are creating a new class and can differentiate it in the interface, we may be able to add a new button. For this feature, we can add a no_think/think checkbox. This may also help us use thinking models more effectively. The field it changes will be a boolean expression, for example like this |
|
Ok, I've uploaded and It works well looks like.
I guess bug fixes are for another time |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Source/AutoTranslation/.vs/AutoTranslation/v17/DocumentLayout.json (1)
1-291: Exclude the.vsdirectory from version control.This is a Visual Studio auto-generated IDE configuration file that stores local developer state (open tabs, scroll positions, timestamps). It should not be committed because:
- Contains machine-specific absolute paths (e.g.,
C:\Program Files (x86)\Steam\...)- Causes unnecessary diff noise and potential merge conflicts
- Has no effect on build or runtime behavior
Add the
.vs/directory to your.gitignore:# Visual Studio .vs/If the
.vsdirectory was committed previously, remove it from tracking:git rm -r --cached .vs/Source/AutoTranslation/APIKeyRotater.cs (1)
25-41: Critical: Race condition with concurrent workers.The array-based rotation uses unsynchronized index increments (
_index = (_index + 1) % keys.Length), which will cause race conditions whenSettings.ConcurrentWorkerCount > 1. Multiple threads accessing theKeyproperty simultaneously can:
- Skip API keys
- Reuse the same key for multiple requests
- Corrupt the index value
This is a critical concurrency bug introduced by the refactoring combined with the new concurrent worker feature.
🔎 Proposed fix using lock-based synchronization
public class APIKeyRotater { private readonly string[] keys; private int _index = 0; + private readonly object _lock = new object(); public APIKeyRotater(IEnumerable<string> keys) { this.keys = keys.Select(key => key.Trim()).ToArray(); if (this.keys.Length == 0) { throw new ArgumentException("No keys provided"); } } public string Key { get { - var key = keys[_index]; - Rotate(); - return key; + lock (_lock) + { + var key = keys[_index]; + Rotate(); + return key; + } } } - public string KeyNoRotate => keys[_index]; + public string KeyNoRotate + { + get + { + lock (_lock) + { + return keys[_index]; + } + } + } public int Count => keys.Length; public void Rotate() { _index = (_index + 1) % keys.Length; } }
🧹 Nitpick comments (2)
Source/AutoTranslation/CacheFileTool.cs (1)
43-43: Consider logging skipped numeric-prefixed keys.The filter correctly prevents XML validation errors (element names cannot start with digits), but silently discarding entries may mask data issues or unexpected key formats. Consider adding a warning log when keys are skipped, similar to the existing error handling pattern on lines 57-62.
🔎 Proposed enhancement
+ var skippedCount = 0; foreach (var (k, v) in lst) { var key = StripInvalidXmlChars(k); if (string.IsNullOrEmpty(key) || string.IsNullOrEmpty(v)) continue; - if (char.IsDigit(key[0])) continue; + if (char.IsDigit(key[0])) + { + skippedCount++; + continue; + } try { e.AppendElement(key, v); } catch (Exception ex) { firstError = firstError ?? ex; errorCount++; } } + if (skippedCount > 0) + { + Log.Warning(AutoTranslation.LogPrefix + $"Skipped {skippedCount} cache entries with numeric-prefixed keys during export."); + }Source/AutoTranslation/TranslatorManager.cs (1)
161-203: Concurrent worker implementation looks good.The new
DoInnerThreadWorkmethod correctly implements per-thread translation processing with configurable sleep time between requests. The sleep time feature addresses the timeout concerns mentioned in the PR discussion for thinking models.Optional: Consider extracting the translation logic (lines 179-196) into a separate method to improve readability and testability, especially if this logic needs to be reused or tested independently.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
1.6/Assemblies/AutoTranslation.dllis excluded by!**/*.dll
📒 Files selected for processing (20)
Languages/English/Keyed/Mod.xmlLanguages/Korean/Keyed/Mod.xmlSource/AutoTranslation/.vs/AutoTranslation/v17/.suoSource/AutoTranslation/.vs/AutoTranslation/v17/DocumentLayout.jsonSource/AutoTranslation/APIKeyRotater.csSource/AutoTranslation/AutoTranslation.csprojSource/AutoTranslation/CacheFileTool.csSource/AutoTranslation/Helpers.csSource/AutoTranslation/Settings.csSource/AutoTranslation/TranslatorManager.csSource/AutoTranslation/Translators/ITranslator.csSource/AutoTranslation/Translators/Translator_BaseAIModel.csSource/AutoTranslation/Translators/Translator_BaseTraditional.csSource/AutoTranslation/Translators/Translator_ChatGPT.csSource/AutoTranslation/Translators/Translator_Claude.csSource/AutoTranslation/Translators/Translator_DeepL.csSource/AutoTranslation/Translators/Translator_Gemini.csSource/AutoTranslation/Translators/Translator_Google.csSource/AutoTranslation/Translators/Translator_Ollama.csSource/AutoTranslation/obj/Debug/AutoTranslation.csproj.AssemblyReference.cache
💤 Files with no reviewable changes (4)
- Source/AutoTranslation/Translators/ITranslator.cs
- Source/AutoTranslation/Translators/Translator_BaseTraditional.cs
- Source/AutoTranslation/Translators/Translator_DeepL.cs
- Source/AutoTranslation/Translators/Translator_Google.cs
🧰 Additional context used
🧬 Code graph analysis (3)
Source/AutoTranslation/Translators/Translator_Gemini.cs (1)
Source/AutoTranslation/Translators/Translator_BaseAIModel.cs (1)
Translator_BaseAIModel(12-147)
Source/AutoTranslation/Translators/Translator_ChatGPT.cs (1)
Source/AutoTranslation/Translators/Translator_BaseAIModel.cs (1)
Translator_BaseAIModel(12-147)
Source/AutoTranslation/Translators/Translator_Claude.cs (1)
Source/AutoTranslation/Translators/Translator_BaseAIModel.cs (1)
Translator_BaseAIModel(12-147)
🔇 Additional comments (18)
Languages/English/Keyed/Mod.xml (1)
22-23: LGTM!The new localization entries for sleep time and concurrent worker settings are well-formed and consistent with the existing translation keys. They correctly use format placeholders for displaying dynamic values.
Source/AutoTranslation/Helpers.cs (2)
63-63: LGTM!Adding
.Trim()improves robustness by normalizing whitespace in JSON-extracted values. This aligns with the cleanup performed inTranslator_Ollama.ParseResponse.
70-70: LGTM!Consistent whitespace normalization for multi-value JSON extraction.
Source/AutoTranslation/Settings.cs (5)
23-24: LGTM!The new concurrency settings fields follow the existing pattern and have sensible defaults (no sleep delay, single worker).
47-48: LGTM!Proper persistence of the new settings using appropriate Scribe keys.
264-264: LGTM!Base class rename from
Translator_BaseOnlineAIModeltoTranslator_BaseAIModelreflects the broader refactoring to support both online and local (Ollama) AI models.
304-308: LGTM!Consistent use of the renamed base class.
338-340: LGTM!The slider ranges are appropriate:
- Sleep time: 0-5000ms allows users to throttle requests to avoid rate limits
- Worker count: 1-5 provides reasonable concurrency without overwhelming the system
Languages/Korean/Keyed/Mod.xml (2)
23-23: LGTM!Korean localization for the concurrent worker count setting is properly formatted and consistent with the English version.
26-26: LGTM!Korean localization for the sleep time setting is properly formatted and consistent with the English version.
Source/AutoTranslation/AutoTranslation.csproj (1)
107-108: LGTM!Project file correctly updated to reflect the translator refactoring: removed the old base class and added the new Ollama translator and renamed base AI model class.
Source/AutoTranslation/Translators/Translator_Claude.cs (1)
13-13: LGTM!The base class change from
Translator_BaseOnlineAIModeltoTranslator_BaseAIModelis part of the systematic refactoring to support both online and local AI models. The new base class maintains API compatibility.Source/AutoTranslation/Translators/Translator_ChatGPT.cs (1)
13-13: LGTM - Base class updated correctly.The inheritance change from
Translator_BaseOnlineAIModeltoTranslator_BaseAIModelaligns with the project-wide refactoring to support both online and local AI models.Source/AutoTranslation/Translators/Translator_Gemini.cs (1)
14-14: LGTM - Base class updated correctly.The inheritance change aligns with the base class refactoring across all AI model translators.
Source/AutoTranslation/Translators/Translator_BaseAIModel.cs (1)
12-15: LGTM - Base class refactoring enables local model support.The rename from
Translator_BaseOnlineAIModeltoTranslator_BaseAIModeland makingRequiresKeyvirtual correctly addresses the architectural need to support both online and local AI models (like Ollama). This resolves the readiness bug mentioned in the PR discussion where local models were incorrectly failing API key validation.Source/AutoTranslation/Translators/Translator_Ollama.cs (2)
20-39: Good: Conditional authorization header for optional API key.The conditional addition of the Authorization header only when
APIKeyis not empty allows Ollama to work both with and without API keys, supporting both secured and unsecured local instances.
93-104: The Unicode unescaping on line 93 is necessary, not redundant. TheGetStringValueFromJson()method only handles escaped quotes and does not unescape Unicode sequences like\u003c,\u003e, and\u000a—these manual replacements are required.Lines 99-100 correctly replace literal backslash-n and backslash-r strings (not actual newline characters), handling cases where the model outputs escaped strings. This pattern aligns with other translators (e.g., Gemini) and reflects the intentional design where the
BasePrompttells the model to preserve Unicode-formatted output. The<think>tag removal and whitespace normalization are appropriate for response cleanup.Source/AutoTranslation/TranslatorManager.cs (1)
53-53: Explicit fallback to Google translator is appropriate.The change from selecting any ready translator to specifically falling back to "Google" provides predictable behavior when the configured translator is unavailable.
| if (ls.ButtonText("AT_Setting_RestartWork".Translate())) | ||
| { | ||
| var t = TranslatorManager.GetTranslator(TranslatorName); | ||
| if (t is Translator_BaseOnlineAIModel aiTranslator) | ||
| if (t is Translator_BaseAIModel aiTranslator) | ||
| { | ||
| aiTranslator.ResetSettings(); | ||
| } | ||
| if (t.Ready) | ||
| { | ||
| TranslatorManager.ClearQueue(); | ||
| TranslatorManager.CurrentTranslator = t; | ||
| TranslatorManager.ClearQueue(); | ||
| TranslatorManager.CurrentTranslator = t; | ||
|
|
||
| InjectionManager.UndoInjectAll(); | ||
| InjectionManager.ClearDefInjectedTranslations(); | ||
| InjectionManager.ReverseTranslator.Clear(); | ||
| InjectionManager.UndoInjectAll(); | ||
| InjectionManager.ClearDefInjectedTranslations(); | ||
| InjectionManager.ReverseTranslator.Clear(); | ||
|
|
||
| InjectionManager.InjectAll(); | ||
| InjectionManager.InjectAll(); | ||
|
|
||
| Messages.Message("AT_Message_RestartWork".Translate(), MessageTypeDefOf.NeutralEvent); | ||
| } | ||
| else | ||
| { | ||
| Messages.Message("AT_Message_RestartFailed".Translate(), MessageTypeDefOf.NegativeEvent); | ||
| } | ||
| Messages.Message("AT_Message_RestartWork".Translate(), MessageTypeDefOf.NeutralEvent); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the context of restart logic and translator state management
echo "=== Searching for RestartWork and related patterns ==="
rg -n "RestartWork" --type=cs -B3 -A15
echo -e "\n=== Searching for ClearQueue usage patterns ==="
rg -n "ClearQueue" --type=cs -B2 -A2
echo -e "\n=== Checking TranslatorManager implementation ==="
fd "TranslatorManager" --type=cs --exec cat {} \;
echo -e "\n=== Checking for Translator Ready state patterns ==="
rg -n "\.Ready|Prepare\(\)" --type=cs -B2 -A2
echo -e "\n=== Checking for previous conditional patterns in InjectionManager ==="
rg -n "InjectionManager\." --type=cs -B2 -A2 | head -100Repository: csh1668/AutoTranslation
Length of output: 10158
🏁 Script executed:
#!/bin/bash
# Check git history for this file to see what was changed
echo "=== Git diff for Settings.cs (recent changes) ==="
git log --oneline -n 20 -- Source/AutoTranslation/Settings.cs
echo -e "\n=== Git show diff for RestartWork context ==="
git log -p -n 5 --follow -- Source/AutoTranslation/Settings.cs | grep -A30 -B5 "RestartWork" | head -100
echo -e "\n=== Check InjectionManager.InjectAll method ==="
rg -A10 "public.*InjectAll\(\)"
echo -e "\n=== Check InjectionManager.UndoInjectAll method ==="
rg -A10 "public.*UndoInjectAll\(\)"
echo -e "\n=== Check ClearQueue full implementation ==="
rg -A5 "public static void ClearQueue"
echo -e "\n=== Check for translator validation/readiness patterns ==="
rg -n "CurrentTranslator\s*=" --type=cs -B2 -A2Repository: csh1668/AutoTranslation
Length of output: 3083
🏁 Script executed:
#!/bin/bash
# Find InjectionManager file and examine InjectAll/UndoInjectAll methods
echo "=== Find InjectionManager implementation ==="
fd "InjectionManager" -t f --type=cs
echo -e "\n=== Check InjectionManager.cs for InjectAll ==="
rg -A15 "void InjectAll\(\)|void UndoInjectAll\(\)"
echo -e "\n=== Check for any validation before InjectAll calls ==="
rg -B5 "InjectAll\(\)" | head -80
echo -e "\n=== Look at the full git diff for the latest commit ==="
git show --stat 83c80de
echo -e "\n=== Check for any translator null validation in RestartWork context ==="
rg -B10 -A20 "if.*t is Translator_BaseAIModel"Repository: csh1668/AutoTranslation
Length of output: 9724
🏁 Script executed:
#!/bin/bash
# Check GetTranslator implementation to see if it can return null
echo "=== Check GetTranslator method signature and implementation ==="
rg -A10 "public static.*GetTranslator\("
echo -e "\n=== Check if null translator is handled elsewhere ==="
rg -B2 -A2 "GetTranslator.*null|GetTranslator.*=="Repository: csh1668/AutoTranslation
Length of output: 961
Add null check for translator assignment.
The translator retrieved via GetTranslator(TranslatorName) can be null (it uses FirstOrDefault()), but is assigned directly to CurrentTranslator without validation. Assign only if the translator is valid, or fall back to a default translator as done in TranslatorManager.Prepare().
| _translationThreads = new Task[Settings.ConcurrentWorkerCount]; | ||
| for (int i = 0; i < Settings.ConcurrentWorkerCount; i++) | ||
| { | ||
| Log.Warning($"Translation thread was killed! {t.Exception?.Message}"); | ||
| }); | ||
| _translationThreads[i] = Task.Factory.StartNew(DoInnerThreadWork).ContinueWith(t => | ||
| { | ||
| Log.Warning($"Translation thread {i} was killed! {t.Exception?.Message}"); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fix closure capture bug in thread completion handlers.
The loop variable i is captured in the closure at line 137, causing all thread completion handlers to log the same thread index (the final value of i after the loop completes). This is a classic closure capture issue in C#.
🔎 Proposed fix
_translationThreads = new Task[Settings.ConcurrentWorkerCount];
for (int i = 0; i < Settings.ConcurrentWorkerCount; i++)
{
- _translationThreads[i] = Task.Factory.StartNew(DoInnerThreadWork).ContinueWith(t =>
+ int threadIndex = i; // Capture loop variable in local scope
+ _translationThreads[i] = Task.Factory.StartNew(DoInnerThreadWork).ContinueWith(t =>
{
- Log.Warning($"Translation thread {i} was killed! {t.Exception?.Message}");
+ Log.Warning($"Translation thread {threadIndex} was killed! {t.Exception?.Message}");
});
}📝 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.
| _translationThreads = new Task[Settings.ConcurrentWorkerCount]; | |
| for (int i = 0; i < Settings.ConcurrentWorkerCount; i++) | |
| { | |
| Log.Warning($"Translation thread was killed! {t.Exception?.Message}"); | |
| }); | |
| _translationThreads[i] = Task.Factory.StartNew(DoInnerThreadWork).ContinueWith(t => | |
| { | |
| Log.Warning($"Translation thread {i} was killed! {t.Exception?.Message}"); | |
| }); | |
| } | |
| _translationThreads = new Task[Settings.ConcurrentWorkerCount]; | |
| for (int i = 0; i < Settings.ConcurrentWorkerCount; i++) | |
| { | |
| int threadIndex = i; // Capture loop variable in local scope | |
| _translationThreads[i] = Task.Factory.StartNew(DoInnerThreadWork).ContinueWith(t => | |
| { | |
| Log.Warning($"Translation thread {threadIndex} was killed! {t.Exception?.Message}"); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In Source/AutoTranslation/TranslatorManager.cs around lines 132 to 139, the
for-loop captures the loop variable i in the ContinueWith closure, causing every
completion handler to log the same (final) index; fix by copying i to a local
variable inside the loop (e.g. int threadIndex = i) and use that local in the
ContinueWith lambda so each handler logs its own thread index, or alternatively
pass the index as state to StartNew/ContinueWith to avoid the closure capture.
| public class Translator_Ollama: Translator_BaseAIModel | ||
| { | ||
| public override string Name => "Ollama"; | ||
| public override string BaseURL => "http://localhost:11434/v1/"; | ||
|
|
||
| protected virtual string RoleSystem => "system"; |
There was a problem hiding this comment.
Override RequiresKey to return false for local Ollama instances.
Since Ollama runs locally and doesn't require an API key by default, this translator should override RequiresKey to return false. The base class now supports this via the virtual keyword (line 15 in Translator_BaseAIModel.cs), which was specifically added to address the readiness bug mentioned in the PR discussion.
🔎 Proposed fix
public class Translator_Ollama: Translator_BaseAIModel
{
public override string Name => "Ollama";
public override string BaseURL => "http://localhost:11434/v1/";
+ public override bool RequiresKey => false;
protected virtual string RoleSystem => "system";Based on learnings from PR comments, PMK95 reported that Ollama wasn't becoming ready due to API key validation, and this override addresses that issue.
🤖 Prompt for AI Agents
In Source/AutoTranslation/Translators/Translator_Ollama.cs around lines 13 to
18, the translator currently lacks an override for RequiresKey which causes
local Ollama instances to be blocked by API key validation; add a public
override bool RequiresKey => false; method to the class so the translator
reports that no API key is required (the base class exposes RequiresKey as
virtual), ensuring readiness checks skip key validation for local Ollama.
| protected override string ParseResponse(string response) | ||
| { | ||
| string content = ""; | ||
|
|
||
| try | ||
| { | ||
| content = response.GetStringValueFromJson("content"); | ||
| } | ||
| catch | ||
| { | ||
| try | ||
| { | ||
| content = response.GetStringValueFromJson("text"); | ||
| } | ||
| catch | ||
| { | ||
| return content; | ||
| } | ||
| } |
There was a problem hiding this comment.
Replace empty catch blocks with explicit exception handling.
The nested try-catch blocks silently swallow all exceptions, making debugging nearly impossible when the response format is unexpected. At minimum, log the exceptions or use specific exception types.
🔎 Proposed fix
protected override string ParseResponse(string response)
{
string content = "";
try
{
content = response.GetStringValueFromJson("content");
}
- catch
+ catch (Exception ex)
{
try
{
content = response.GetStringValueFromJson("text");
}
- catch
+ catch (Exception ex2)
{
+ Log.Warning(AutoTranslation.LogPrefix + $"{Name}: Failed to parse response. First attempt (content): {ex.Message}, Second attempt (text): {ex2.Message}");
return content;
}
}🤖 Prompt for AI Agents
In Source/AutoTranslation/Translators/Translator_Ollama.cs around lines 70-88,
the nested try/catch blocks currently swallow all exceptions; replace the empty
catch blocks with explicit exception handling that catches specific JSON/parsing
exceptions (e.g., JsonException, FormatException, ArgumentNullException) or a
single catch (Exception ex) if specific types aren't available, log the
exception (using the translator's logger or a provided logging facility)
including the exception message/stack, and then continue to the next parsing
attempt or return the content as before; ensure the first catch logs the failure
to get "content" and the inner catch logs the failure to get "text" before
returning.
3 participants
This PR adds Ollama to the Auto Translation project.
Ollama uses an OpenAI-compatible API, but when I selected ChatGPT as the provider and entered my local address as a custom URL, I noticed that the system prompt wasn't working correctly. Other than that, requests were being sent and responses were received, but the responses were dirty. Both characters like \n were coming through, and in thinking models, tags were appearing. I prepared this work by cleaning these up and tested it with working models I have (gemma3:27b, qwen3 30b-a3b, phi4-reasoning). If I haven't missed anything, it seems to be working correctly.
My C# knowledge was 0 before this project and may have increased to 0.1 now. Honestly, I wrote this without fully understanding how the code works and proceeded by copying existing code. I may have made a faulty extension or some things might be duplicated. If you point these out, I'll try to fix them, but honestly I'm not very enthusiastic about it.
It might also be useful to add an example prompt to the project. For example:
You are a translation AI trained to translate the text of the video game RimWorld into Turkish. Your task is to translate the English text provided to you into Turkish while remaining faithful to the atmosphere and terminology of the game.
When translating, pay attention to the following rules:
Tone and Style: RimWorld has a unique narrative style that is generally dark, somewhat detached, technical, and occasionally darkly humorous. Ensure your translations reflect this style. Avoid using overly emotional or literary language.
Terminology: Consistently translate game-specific names (e.g., Plasteel, Mechanoid, Thrumbo, Scyther), technology names, and mechanics. If there is a community-accepted translation for a term, try to use it.
Simplicity and Clarity: Even if the sentences are technical, use Turkish that is easy for the player to understand and flows smoothly.
Translation Only: In your response, write only the Turkish translation of the requested text. Do not add any explanations, greetings, or additional information.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.