fix: resolve overall progress calculation and improve UI responsiveness #22
Conversation
- Remove clear_logs invocation when starting translation - Preserve translation history across multiple translation sessions - Users can now review logs from previous translations
- Remove automatic log reset when isTranslating becomes true - Users can now view logs from previous translations in the same session - Log history is preserved until manually cleared with Clear Logs button
- Remove LogViewer's useEffect that cleared logs when isTranslating changed - Add useEffect to reload logs from backend when dialog opens - This ensures logs are visible even after translation completes
- Clear in-memory log buffer when starting new translation session - Remove redundant log reload when dialog opens - Keep logs visible after translation completes - Each session gets its own log file while in-memory buffer is cleared for new session
- Remove log clearing when starting new translation session - Remove auto-close behavior for log dialog after translation completion - Logs now persist across translation sessions until manually cleared - Users can see translation history accumulated across multiple sessions
- Clear logs when starting new translation session (log viewer initialization) - Preserve logs after translation completion until next session starts - Restore auto-close behavior for log dialog after translation completes - Each session has clean log viewer start with accumulated logs during session
- Create UnifiedLogViewer component combining real-time and historical log viewing - Support both 'realtime' mode for current translation and 'historical' mode for past sessions - Enhanced historical log viewer with better formatting, filtering, and larger width - Maintain all advanced features from real-time viewer (auto-scroll, filtering, styling) - Replace simple text-based historical log viewer with full-featured component - Backward compatibility maintained through wrapper components
- Add useCallback to filterLogs function with isTranslating dependency - Add useCallback to parseRawLogContent function - Add useMemo to filteredLogs and dialogTitle calculations - Prevent Maximum update depth exceeded error during translation
- Add detect_snbt_content_type to imports in lib.rs - Register detect_snbt_content_type in invoke_handler - Resolves 'Command detect_snbt_content_type not found' error
- Remove Clear Logs button from UnifiedLogViewer - Adjust footer layout for better spacing - Keep only Close button and Auto Scroll checkbox (realtime mode only)
- Add debug logs to translation-runner.ts to track summary update calls - Add debug logs to backup.rs update_translation_summary function - Track sessionId, profileDirectory, translatedKeys, totalKeys values - Help identify why translation summaries are not being generated
- Move sessionId generation outside try block to fix scope issue - sessionId is now accessible in the onTranslate function call - Resolves 'Can't find variable: sessionId' error
- Remove individual job summary updates during processing - Add batch summary update at the end of runTranslationJobs - Reduces excessive file writes from once per job to once per translation session - Maintains same summary data but improves performance
- Add batch_update_translation_summary function to handle multiple entries at once - Replace N individual calls with single batch call at translation completion - Reduces file I/O from N writes to 1 write per translation session - Maintains same summary data structure and functionality
- Set file name column width to 200px with max 300px - Add truncate styling with title hover for full file names - Reduces horizontal space usage while maintaining readability - Improves table layout balance in translation history dialog
- Remove condition that prevented log updates when existing logs present - Always refresh logs to reflect current backend state - Add explicit log refresh when translation starts (isTranslating becomes true) - Fix timing issue where frontend logs weren't updated after backend clear_logs
…nsiveness - Fix overall progress calculation in all translation tabs (mods, quests, guidebooks, custom files) - Change denominator from selected items to actual jobs to ensure 100% completion - Improve translation history dialog table responsiveness with flexible column widths - Remove unused variables and fix TypeScript/ESLint issues - Apply Rust code formatting for consistency - Add proper progress tracking for skipped translations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
WalkthroughThis update introduces session-based logging and translation workflows, enhances SNBT content detection, and unifies log viewing across the frontend. It refactors translation tab interfaces to propagate session IDs, updates backend commands for batch translation summary updates, and adds comprehensive tests for FTB quest translation, SNBT content detection, and realistic Minecraft directory structures. Mock utilities and translation logic are improved for accurate, realistic testing. Changes
Sequence Diagram(s)Session-based Translation FlowsequenceDiagram
participant User
participant TranslationTab
participant Backend (Tauri)
participant UnifiedLogViewer
User->>TranslationTab: Select directory and targets
TranslationTab->>Backend (Tauri): Generate sessionId
TranslationTab->>Backend (Tauri): Create logs directory (sessionId)
TranslationTab->>TranslationTab: Start translation (sessionId)
TranslationTab->>Backend (Tauri): Run translation jobs (sessionId)
Backend (Tauri)-->>TranslationTab: Translation results & logs
TranslationTab->>Backend (Tauri): Batch update translation summary (sessionId)
User->>UnifiedLogViewer: View logs (sessionId)
UnifiedLogViewer->>Backend (Tauri): Fetch logs (sessionId)
Backend (Tauri)-->>UnifiedLogViewer: Log data
SNBT Content Detection and TranslationsequenceDiagram
participant TranslationRunner
participant Backend (Tauri)
participant FileSystem
TranslationRunner->>Backend (Tauri): detect_snbt_content_type(filePath)
Backend (Tauri)->>FileSystem: Read SNBT file
FileSystem-->>Backend (Tauri): SNBT content
Backend (Tauri)-->>TranslationRunner: Content type (direct_text/json_keys)
alt direct_text
TranslationRunner->>FileSystem: Overwrite SNBT file with translation
else json_keys
TranslationRunner->>FileSystem: Write translation to language-suffixed file
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
src/lib/services/file-service.ts (1)
93-99: LGTM: Improved mock with realistic directory paths.The updated mock now returns platform-specific, realistic Minecraft directory paths, which better supports testing and aligns with the backend changes that removed the
"NATIVE_DIALOG:"prefix.However, there's a static analysis issue that should be addressed:
case "open_directory_dialog": - // Return a realistic test minecraft path for development - console.log("[MOCK] Simulating native dialog selection"); - // Use a path that resembles actual Minecraft installations - const testPath = process.platform === 'win32' - ? 'C:\\Users\\Test\\AppData\\Roaming\\.minecraft' - : '/home/test/.minecraft'; - return testPath as unknown as T; + { + // Return a realistic test minecraft path for development + console.log("[MOCK] Simulating native dialog selection"); + // Use a path that resembles actual Minecraft installations + const testPath = process.platform === 'win32' + ? 'C:\\Users\\Test\\AppData\\Roaming\\.minecraft' + : '/home/test/.minecraft'; + return testPath as unknown as T; + }This wraps the declaration in a block to prevent potential access from other switch cases, addressing the static analysis warning.
src-tauri/src/minecraft/mod.rs (1)
944-979: Consider enhancing the content type detection logic.The current heuristic approach works but could be improved:
- The function might misclassify files that contain both JSON keys and direct text
- Consider adding SNBT format validation before content analysis
- The pattern matching could be more comprehensive
Consider this enhanced approach that checks for the predominant content type:
pub async fn detect_snbt_content_type(file_path: &str) -> Result<String, String> { use std::fs; let content = fs::read_to_string(file_path).map_err(|e| format!("Failed to read SNBT file: {e}"))?; + // Basic SNBT format validation + if !content.trim_start().starts_with('{') || !content.trim_end().ends_with('}') { + return Err("File does not appear to be valid SNBT format".to_string()); + } + // Simple heuristic: check if the content contains typical JSON key patterns // JSON keys usually contain dots (e.g., "item.minecraft.stick") or colons (e.g., "minecraft:stick") - let has_json_key_patterns = content.contains("minecraft:") + let json_key_count = content.matches("minecraft:").count() - || content.contains("ftbquests:") + + content.matches("ftbquests:").count() - || content.contains(".minecraft.") + + content.matches(".minecraft.").count() - || content.contains("item.") + + content.matches("item.").count() - || content.contains("block.") + + content.matches("block.").count() - || content.contains("entity.") + + content.matches("entity.").count() - || content.contains("gui.") + + content.matches("gui.").count() - || content.contains("quest."); + + content.matches("quest.").count(); // Check if the content has direct readable text (not just IDs and keys) // Look for typical quest text patterns - let has_direct_text = content.contains("description:") + let direct_text_count = content.matches("description:").count() - || content.contains("title:") + + content.matches("title:").count() - || content.contains("subtitle:") + + content.matches("subtitle:").count() - || content.contains("text:"); + + content.matches("text:").count(); - if has_json_key_patterns && !has_direct_text { + // Determine predominant content type based on counts + if json_key_count > 0 && direct_text_count == 0 { Ok("json_keys".to_string()) - } else if has_direct_text { + } else if direct_text_count > 0 && json_key_count < direct_text_count { Ok("direct_text".to_string()) + } else if json_key_count >= direct_text_count && json_key_count > 0 { + Ok("json_keys".to_string()) } else { // Default to direct_text if uncertain Ok("direct_text".to_string()) } }src/__tests__/integration/realistic-minecraft-directory.test.ts (1)
183-226: Consider simplifying the mock to focus on testing behavior rather than reimplementing logic.The current mock implementation duplicates the SNBT content detection logic. Consider simplifying to test the command invocation and response handling.
it('should properly analyze direct text SNBT content', async () => { mockInvoke.mockImplementation((command: string, args: any) => { if (command === 'detect_snbt_content_type') { - const fs = require('fs'); - if (fs.existsSync(args.filePath)) { - const content = fs.readFileSync(args.filePath, 'utf-8'); - - // Simple heuristic: check for direct text vs JSON keys - const hasDirectText = content.includes('Welcome to') || - content.includes('Time to') || - content.includes('Complete this quest'); - const hasJsonKeys = content.includes('ftbquests.') || - content.includes('minecraft:') || - content.includes('item.'); - - if (hasJsonKeys && !hasDirectText) { - return Promise.resolve('json_keys'); - } else if (hasDirectText) { - return Promise.resolve('direct_text'); - } - } - return Promise.resolve('direct_text'); + // Mock the expected response based on the file path + if (args.filePath.includes('getting_started.snbt')) { + return Promise.resolve('direct_text'); + } + return Promise.resolve('json_keys'); } return Promise.resolve('direct_text'); });src/components/tabs/guidebooks-tab.tsx (1)
277-296: Good fix for progress calculation! Consider adding error handling.The change to use
jobs.lengthcorrectly ensures progress reaches 100% when all translatable guidebooks are complete. However, the session directory creation could benefit from error handling.- // Use the session ID provided by the common translation tab - const minecraftDir = selectedDirectory; - const sessionPath = await invoke<string>('create_logs_directory_with_session', { - minecraftDir: minecraftDir, - sessionId: sessionId - }); - console.log(`Guidebooks translation session created: ${sessionPath}`); + // Use the session ID provided by the common translation tab + const minecraftDir = selectedDirectory; + let sessionPath: string; + try { + sessionPath = await invoke<string>('create_logs_directory_with_session', { + minecraftDir: minecraftDir, + sessionId: sessionId + }); + console.log(`Guidebooks translation session created: ${sessionPath}`); + } catch (error) { + console.error('Failed to create session directory:', error); + // Consider whether to continue or abort translation + throw new Error(`Failed to create session directory: ${error}`); + }src/__tests__/test-utils/minecraft-directory-mock.ts (1)
22-22: Improve random ID generation for better uniqueness.The current random ID generation uses a predictable pattern that could lead to collisions in tests.
Apply this diff to use a more robust random ID generation:
- const basePath = join(tmpdir(), `minecraft-test-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`); + const basePath = join(tmpdir(), `minecraft-test-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`);Note: Also consider using
crypto.randomUUID()for thegenerateRandomIdfunction if available.src/components/ui/unified-log-viewer.tsx (1)
138-273: Log filtering logic is comprehensive but could be simplified.The filtering logic handles various log types appropriately, but the extensive conditional chains could benefit from refactoring for better maintainability.
Consider extracting the filtering logic into separate functions:
const shouldShowErrorLogs = (log: LogEntry) => { const levelStr = getLogLevelString(log.level).toLowerCase(); return levelStr === 'error' || log.message.includes('[ErrorLogger]') || log.message.includes('TranslationService.logError'); }; const shouldShowTranslationLogs = (log: LogEntry, isTranslating: boolean) => { const translationTypes = ['TRANSLATION', 'TRANSLATION_START', 'TRANSLATION_STATS', 'TRANSLATION_PROGRESS', 'TRANSLATION_COMPLETE']; return isTranslating && translationTypes.includes(log.process_type || ''); };src/lib/services/__tests__/ftb-quest-realistic.e2e.test.ts (1)
119-119: Fix switch case variable declarations.The static analysis correctly identifies that variables declared in switch cases should be wrapped in blocks to prevent access from other cases. While the current code works due to return statements, it's better to follow best practices:
case 'read_text_file': { const content = mockFileStructure[args.path as keyof typeof mockFileStructure]; return Promise.resolve(content || ''); }This applies to lines 119, 218-219, and 304-307.
Also applies to: 218-219, 304-307
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
public/locales/en/common.json(1 hunks)public/locales/ja/common.json(1 hunks)src-tauri/src/backup.rs(3 hunks)src-tauri/src/filesystem.rs(5 hunks)src-tauri/src/lib.rs(4 hunks)src-tauri/src/minecraft/mod.rs(2 hunks)src/__tests__/components/translation-tab.test.tsx(2 hunks)src/__tests__/integration/realistic-minecraft-directory.test.ts(1 hunks)src/__tests__/tabs/mods-tab.test.tsx(3 hunks)src/__tests__/test-utils/minecraft-directory-mock.ts(1 hunks)src/components/tabs/common/translation-tab.tsx(6 hunks)src/components/tabs/custom-files-tab.tsx(3 hunks)src/components/tabs/guidebooks-tab.tsx(5 hunks)src/components/tabs/mods-tab.tsx(3 hunks)src/components/tabs/quests-tab.tsx(8 hunks)src/components/ui/log-dialog.tsx(1 hunks)src/components/ui/translation-history-dialog.tsx(8 hunks)src/components/ui/unified-log-viewer.tsx(1 hunks)src/lib/services/__tests__/ftb-quest-logic.e2e.test.ts(1 hunks)src/lib/services/__tests__/ftb-quest-realistic.e2e.test.ts(1 hunks)src/lib/services/__tests__/snbt-content-detection.test.ts(1 hunks)src/lib/services/__tests__/snbt-content-unit.test.ts(1 hunks)src/lib/services/file-service.ts(1 hunks)src/lib/services/translation-runner.ts(2 hunks)src/lib/test-utils/mock-snbt-files.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
src/__tests__/components/translation-tab.test.tsx (1)
src/lib/services/file-service.ts (1)
FileService(192-386)
src/__tests__/tabs/mods-tab.test.tsx (1)
src/lib/services/file-service.ts (1)
FileService(192-386)
src/lib/services/translation-runner.ts (3)
src/lib/store/index.ts (1)
useAppStore(120-444)src/lib/services/file-service.ts (1)
invoke(206-213)src-tauri/src/logging.rs (1)
error(145-147)
src/lib/services/__tests__/ftb-quest-logic.e2e.test.ts (3)
src/lib/services/file-service.ts (2)
invoke(206-213)FileService(192-386)src/lib/services/translation-service.ts (1)
TranslationService(88-1017)src/lib/services/translation-runner.ts (1)
runTranslationJobs(31-203)
src/components/ui/log-dialog.tsx (1)
src/components/ui/unified-log-viewer.tsx (1)
UnifiedLogViewer(41-556)
src/components/tabs/custom-files-tab.tsx (1)
src/lib/services/file-service.ts (1)
invoke(206-213)
src/components/tabs/common/translation-tab.tsx (1)
src/lib/services/file-service.ts (1)
invoke(206-213)
src/components/tabs/guidebooks-tab.tsx (1)
src/lib/services/file-service.ts (1)
invoke(206-213)
src/components/tabs/quests-tab.tsx (1)
src/lib/services/file-service.ts (1)
FileService(192-386)
🪛 Biome (1.9.4)
src/lib/services/file-service.ts
[error] 96-98: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
src/lib/services/__tests__/ftb-quest-realistic.e2e.test.ts
[error] 119-119: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 218-219: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 304-307: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
src/components/ui/unified-log-viewer.tsx
[error] 416-416: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test
- GitHub Check: Security Scan
- GitHub Check: Backend Tests
🔇 Additional comments (45)
public/locales/en/common.json (1)
205-205: LGTM: Translation key addition is correct.The new
"clearLogs"key is appropriately placed in the"logs"section and follows the existing naming convention. This supports the UI improvements mentioned in the PR objectives.src/__tests__/components/translation-tab.test.tsx (1)
129-129: LGTM: Test mocks updated to align with backend changes.The removal of the
"NATIVE_DIALOG:"prefix from the mock return values correctly reflects the updated backend behavior insrc-tauri/src/filesystem.rswhere theopen_directory_dialogfunction now returns directory paths without this prefix.Also applies to: 345-345
public/locales/ja/common.json (1)
205-205: LGTM: Japanese translation addition is correct.The Japanese translation
"ログをクリア"for"clearLogs"is appropriate and maintains consistency with the English locale file addition. This supports proper internationalization of the clear logs functionality.src/__tests__/tabs/mods-tab.test.tsx (1)
113-113: LGTM: Test mocks updated to align with backend changes.The removal of the
"NATIVE_DIALOG:"prefix from the mock return values correctly reflects the updated backend behavior whereopen_directory_dialognow returns directory paths without this prefix.Also applies to: 165-165, 351-351
src-tauri/src/lib.rs (1)
12-13: LGTM! Clean integration of new Tauri commands.The new commands
batch_update_translation_summaryanddetect_snbt_content_typeare properly imported and added to the invoke handler, maintaining consistency with the existing pattern.Also applies to: 30-31, 130-131
src/components/tabs/mods-tab.tsx (1)
203-204: Excellent improvements to session handling and progress calculation.The changes correctly:
- Accept
sessionIdfrom the parent component, following proper separation of concerns- Fix progress calculation to use
jobs.length(actual translatable items) instead ofsortedTargets.length(which includes skipped items)- Use the provided sessionId for logs directory creation
These changes align well with the PR objective of fixing progress calculation issues.
Also applies to: 301-301, 311-316
src/lib/services/__tests__/snbt-content-detection.test.ts (1)
1-252: Excellent test coverage for SNBT content detection.The test suite is well-structured and comprehensive:
- Clear separation between unit and integration tests
- Proper mock setup and cleanup
- Good coverage of edge cases and error scenarios
- Parameterized tests for various content patterns
- Integration tests properly simulate the full translation flow
src/components/ui/log-dialog.tsx (1)
1-20: Clean refactoring to use UnifiedLogViewer.Excellent simplification that:
- Eliminates code duplication by delegating to UnifiedLogViewer
- Maintains backward compatibility through re-export
- Correctly sets mode="realtime" for the LogDialog use case
- Properly imports React for JSX usage
src-tauri/src/minecraft/mod.rs (1)
896-908: LGTM! Clear logic with helpful documentation.The change to exclude SNBT files from translation existence checks is well-documented and aligns with the in-place translation strategy for SNBT files.
src/lib/services/__tests__/snbt-content-unit.test.ts (1)
1-242: Excellent test coverage!The unit tests comprehensively cover SNBT content detection scenarios with well-structured test cases and clear assertions.
src/lib/services/__tests__/ftb-quest-logic.e2e.test.ts (1)
1-469: Well-structured E2E tests with comprehensive coverage!The tests effectively validate the complete FTB quest translation workflow, including KubeJS lang files, SNBT content detection, and translation summary updates.
src/components/tabs/custom-files-tab.tsx (4)
170-171: Function signature enhancement aligns with session-based logging architecture.The addition of
sessionIdparameter tohandleTranslatefunction is well-implemented and consistent with the broader session-based logging refactor mentioned in the PR objectives.
274-278: Progress tracking fix correctly addresses the reported issue.Moving the total file count calculation after job creation ensures progress tracking uses actual translatable jobs rather than all selected targets (including potentially skipped ones). This directly addresses the PR objective of fixing progress calculation that was stalling around 60-70%.
279-286: Session-based logging integration is properly implemented.The session logging directory creation follows the established pattern from other translation tabs and properly handles the session context. The console logging provides useful debugging information.
296-296: Session ID propagation maintains consistency across the translation workflow.The
sessionIdparameter is correctly passed torunTranslationJobsto maintain session context throughout the translation process.src-tauri/src/filesystem.rs (4)
168-177: Function refactoring enhances quest file filtering capabilities.The introduction of
get_ftb_quest_files_with_languageand refactoring of the original function to delegate to it is a clean design pattern that maintains backward compatibility while adding new functionality.
237-247: Target language existence check prevents redundant translations.The logic to skip
en_us.jsonwhen a target language file already exists is a smart optimization that prevents unnecessary duplicate processing. This improves efficiency in translation workflows.
340-343: SNBT file processing logic simplification is consistent with translation strategy.The removal of language-suffix skipping for SNBT files aligns with the AI summary's mention that "SNBT files are always translated in-place and language suffix detection is limited to JSON files." This change makes the code more predictable and consistent.
727-728: Directory path cleaning removes unnecessary prefix handling.The removal of
"NATIVE_DIALOG:"prefix handling simplifies the code and aligns with the frontend changes that no longer expect this prefix. This cleanup improves maintainability.src/__tests__/test-utils/minecraft-directory-mock.ts (3)
10-26: Well-designed interface for test directory management.The
MinecraftTestStructureinterface provides a clean API with proper cleanup functionality. The structure mirrors real Minecraft directory layouts effectively.
131-201: SNBT content generation functions create realistic test data.The
createRealisticQuestSNBTandcreateRealisticRewardSNBTfunctions generate well-structured SNBT content that closely mimics real FTB quest files. The use of realistic quest structures with proper dependency chains enhances test validity.
206-236: Translation file addition supports comprehensive localization testing.The
addTranslatedFilesfunction properly creates language-specific files for both KubeJS and Better Questing formats, enabling thorough testing of translation detection and handling logic.src/components/ui/unified-log-viewer.tsx (3)
25-48: Component interface design effectively supports dual log viewing modes.The props interface cleanly separates concerns between real-time and historical log viewing with appropriate optional parameters. The mode-based approach is intuitive and maintainable.
276-310: Historical log parsing handles multiple log formats robustly.The parsing logic accommodates both detailed and simple log formats with proper fallback handling. The regex patterns effectively extract timestamp, level, and message components.
421-433: User interaction detection prevents unwanted auto-scroll interruption.The timeout-based approach to detect when user interaction ends is well-implemented and provides a good user experience by respecting manual scrolling while maintaining auto-scroll functionality.
src/lib/test-utils/mock-snbt-files.ts (5)
6-108: Direct text SNBT mocks provide realistic quest file structures.The mock files accurately represent FTB quest structure with proper SNBT syntax, realistic quest dependencies, and comprehensive quest elements including tasks, rewards, and descriptions. This will enable thorough testing of direct text translation workflows.
111-223: JSON key reference mocks effectively test localization key translation.The mock files with localization keys (
ftbquests.*) properly simulate how modern quest packs use language files for internationalization. The mixed content example is particularly valuable for testing complex translation scenarios.
226-252: KubeJS language file mock supports comprehensive localization testing.The mock language file contains realistic localization keys that correspond to the quest file references, enabling end-to-end testing of the KubeJS translation workflow.
254-299: Expected translations provide accurate test validation data.The Japanese translation mappings are well-structured and provide realistic translation examples that will enable precise test assertions for both direct text and localization key translation workflows.
301-309: File structure mapping enables integration testing.The path-to-content mapping provides a clean way to simulate a complete Minecraft directory structure for integration tests, supporting comprehensive validation of file discovery and translation workflows.
src/components/tabs/common/translation-tab.tsx (4)
101-102: LGTM!The addition of the
sessionIdparameter to theonTranslatecallback signature is well-implemented and aligns with the session-based logging architecture.
186-189: Good simplification!Removing the "NATIVE_DIALOG:" prefix handling and consolidating the debug logging improves code clarity and maintainability.
Also applies to: 213-214
333-385: Well-structured session management implementation!The session ID generation and propagation through the translation workflow is properly implemented. The order of operations ensures clean session initialization.
468-468: Consistent UI display update.The direct display of
profileDirectoryaligns with the simplified directory handling approach.src/components/tabs/quests-tab.tsx (5)
215-216: Consistent session ID implementation!The
sessionIdparameter is properly integrated and used for session-specific logs directory creation, maintaining consistency with the common translation tab.Also applies to: 230-235
288-304: Good implementation of SNBT content type detection!The content type detection logic is well-structured with proper error handling and logging. The fallback to "direct_text" ensures robustness.
341-344: Critical progress calculation fix!Using
jobs.lengthinstead ofsortedTargets.lengthcorrectly excludes skipped translations from the progress calculation, ensuring progress reaches 100% upon completion. This addresses the main issue described in the PR.
369-378: Correct KubeJS lang file handling!The special handling for KubeJS language files is properly implemented. Creating a new target language file instead of modifying the original
en_us.jsonfollows the expected pattern for Minecraft language files.
421-436: Proper file output handling!The distinction between SNBT files (in-place modification) and other files (language suffix) is correctly implemented. The debug logging helps track the output decisions.
src/components/ui/translation-history-dialog.tsx (3)
13-13: Clean integration of UnifiedLogViewer!The refactoring to use
UnifiedLogViewercomponent simplifies the code by removing manual log fetching and display logic. The component properly passes all required props including mode, sessionId, and minecraftDir.Also applies to: 379-382, 523-531
150-164: Excellent responsive table design!The flexible column widths with min/max constraints and percentage-based sizing improve the UI responsiveness. The 50% width allocation for the file name column appropriately prioritizes the most important information.
Also applies to: 468-468, 472-486
302-303: Consistent directory path handling!The removal of "NATIVE_DIALOG:" prefix handling maintains consistency with the simplification across other components.
Also applies to: 437-437
src-tauri/src/backup.rs (2)
363-375: Improved error handling for translation summaries!The distinction between a missing summary file (returns empty summary) and a missing session directory (returns error) provides better user feedback and handles the edge case where a session exists but no translations have been completed yet.
457-554: Well-implemented batch update function!The
batch_update_translation_summaryfunction is properly structured with:
- Good error handling for missing or malformed fields
- Default values for safety
- Consistent JSON serialization using
serialize_json_sorted- Proper session directory creation
This supports the performance optimization of batch updating summaries after all translations complete.
src/lib/services/__tests__/ftb-quest-realistic.e2e.test.ts (1)
1-441: Comprehensive and well-structured E2E test suite!The test suite provides excellent coverage for FTB Quest translation logic with:
- Realistic SNBT file content and scenarios
- Proper mocking of Tauri API calls
- Good organization with describe blocks
- Coverage of both direct text and JSON key scenarios
|
|
||
| // Cleanup | ||
| return () => { | ||
| unlistenPromise.then(unlisten => unlisten && unlisten()); |
There was a problem hiding this comment.
Fix unsafe optional chaining as identified by static analysis.
The static analysis tool correctly identified an unsafe optional chaining pattern.
Apply this diff to fix the unsafe optional chaining:
- unlistenPromise.then(unlisten => unlisten && unlisten());
+ unlistenPromise.then(unlisten => unlisten?.());📝 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.
| unlistenPromise.then(unlisten => unlisten && unlisten()); | |
| unlistenPromise.then(unlisten => unlisten?.()); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 416-416: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In src/components/ui/unified-log-viewer.tsx at line 416, the use of optional
chaining with unlistenPromise.then(unlisten => unlisten && unlisten()) is
unsafe. To fix this, avoid optional chaining by explicitly checking if unlisten
is a function before calling it. Replace the current line with a safe
conditional call that ensures unlisten is defined and callable before invoking
it.
src-tauri/src/backup.rs
Outdated
| println!("[update_translation_summary] Called with: minecraft_dir={}, session_id={}, translation_type={}, name={}, status={}, translated_keys={}, total_keys={}, target_language={}", | ||
| minecraft_dir, session_id, translation_type, name, status, translated_keys, total_keys, target_language); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using proper logging instead of println!
While the debug output is helpful, using println! in production code isn't ideal. Consider using the existing AppLogger for these debug messages, similar to how it's used in other functions in this file.
logger.debug(&format!("[update_translation_summary] Called with: minecraft_dir={}, session_id={}, ...",
minecraft_dir, session_id, ...), Some("BACKUP"));Also applies to: 409-412, 450-453, 465-466, 473-476, 548-552
🤖 Prompt for AI Agents
In src-tauri/src/backup.rs around lines 401 to 402, replace the println! macro
with the AppLogger's debug method to log the message properly. Use logger.debug
with a formatted string containing the same variables and include the log
category "BACKUP" as the second argument. Apply the same change to the other
specified line ranges (409-412, 450-453, 465-466, 473-476, 548-552) to ensure
consistent logging throughout the file.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
39-42: Commented-out tmate block clutters the workflowThe tmate step is now disabled but left in the file as commented YAML. Unless you foresee re-enabling it soon, consider removing the block (or gating it behind an input variable) to keep the workflow concise and avoid confusion for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build.yml(1 hunks)jest.setup.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Backend Tests
🔇 Additional comments (1)
jest.setup.js (1)
5-7: LGTM! Good addition of complementary Tauri mock.The addition of
jest.fn()towindow.__TAURI_INTERNALS__.invokeproperly complements the existingglobal.__TAURI__.invokemock. This ensures comprehensive test coverage for both Tauri API access patterns and supports the new test suites mentioned in the PR objectives.
Replace direct Tauri invoke mocks with FileService.setTestInvokeOverride method for more reliable test execution. This ensures the FileService internal tauriInvoke function uses the test mock properly. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add backend support for checking existing translations in mod JAR files - Implement `check_mod_translation_exists` Tauri command with comprehensive tests - Add debug component for testing translation detection functionality - Fix placeholder replacement bug in completion dialog (completed/total params) - Add support for tracking existing translations in mod scan results - Improve translation targeting with pre-translation existence checks - Add integration tests for mod translation flow - Fix Rust formatting issues to pass CI validation This enhancement allows the application to detect which mods already have translations, enabling smarter translation workflows and preventing unnecessary API calls. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src-tauri/tests/mod_translation_integration.rs (1)
148-149: Inconsistent string literal usageLine 148 uses a regular string with UTF-8 characters while other similar lines use byte string literals. This inconsistency could lead to encoding issues.
- zip.write_all(r#"{"item.complexmod.item": "複雑なアイテム"}"#.as_bytes()) + zip.write_all(br#"{"item.complexmod.item": "複雑なアイテム"}"#)docs/translation-detection-fix-plan.md (1)
77-91: Add language specification to fenced code blockThe code block is missing a language identifier, which helps with syntax highlighting and readability.
- ``` + ```bash unzip -l mod.jar | grep -E "assets/.*/lang/" ```src-tauri/src/minecraft/mod_translation_test.rs (1)
344-348: Consider more flexible performance thresholdThe 1-second threshold might be too strict for CI environments or slower systems. Consider making it configurable or more lenient.
- assert!( - duration.as_millis() < 1000, - "Should complete within 1 second even for large JARs" - ); + assert!( + duration.as_secs() < 5, + "Should complete within 5 seconds even for large JARs (took {}ms)", + duration.as_millis() + );src/components/tabs/common/translation-tab.tsx (1)
304-336: Good UX for existing translation handlingThe pre-check implementation provides excellent user feedback with appropriate warnings and early exit optimization.
Consider parallel checking for better performance with many mods:
const existenceChecks = selectedTargets.map(async (target) => { try { return await FileService.invoke<boolean>("check_mod_translation_exists", { modPath: target.path, modId: target.id, targetLanguage: targetLanguage }); } catch (error) { console.error(`Failed to check existing translation for ${target.name}:`, error); return false; } }); const results = await Promise.all(existenceChecks); const existingCount = results.filter(exists => exists).length;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
docs/translation-detection-fix-plan.md(1 hunks)public/locales/en/common.json(3 hunks)public/locales/ja/common.json(3 hunks)src-tauri/src/lib.rs(4 hunks)src-tauri/src/minecraft/debug_translation_check.rs(1 hunks)src-tauri/src/minecraft/mod.rs(2 hunks)src-tauri/src/minecraft/mod_translation_test.rs(1 hunks)src-tauri/tests/mod_translation_integration.rs(1 hunks)src/__tests__/integration/mod-translation-flow.test.ts(1 hunks)src/__tests__/services/mod-translation-check.test.ts(1 hunks)src/components/debug/translation-check-debug.tsx(1 hunks)src/components/tabs/common/translation-tab.tsx(8 hunks)src/components/tabs/guidebooks-tab.tsx(8 hunks)src/components/tabs/mods-tab.tsx(9 hunks)src/components/tabs/quests-tab.tsx(12 hunks)src/components/ui/completion-dialog.tsx(1 hunks)src/lib/types/minecraft.ts(1 hunks)test-summary.md(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/lib/types/minecraft.ts
- test-summary.md
- src-tauri/src/minecraft/mod.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- public/locales/en/common.json
- public/locales/ja/common.json
- src/components/tabs/quests-tab.tsx
- src/components/tabs/guidebooks-tab.tsx
- src-tauri/src/lib.rs
🧰 Additional context used
🧬 Code Graph Analysis (2)
src-tauri/src/minecraft/debug_translation_check.rs (1)
src-tauri/src/minecraft/mod.rs (1)
check_mod_translation_exists(846-881)
src/components/tabs/common/translation-tab.tsx (3)
src/lib/types/minecraft.ts (2)
TranslationTarget(103-122)TranslationResult(144-163)src/lib/services/translation-service.ts (1)
TranslationService(88-1017)src/lib/services/file-service.ts (2)
FileService(192-386)invoke(206-213)
🪛 Biome (1.9.4)
src/__tests__/integration/mod-translation-flow.test.ts
[error] 77-77: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 78-78: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
src/__tests__/services/mod-translation-check.test.ts
[error] 215-252: Do not export from a test file.
(lint/suspicious/noExportsInTest)
🪛 markdownlint-cli2 (0.17.2)
docs/translation-detection-fix-plan.md
88-88: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build - aarch64-apple-darwin
- GitHub Check: Build - x86_64-unknown-linux-gnu
- GitHub Check: Build - x86_64-apple-darwin
- GitHub Check: Build - x86_64-pc-windows-msvc
- GitHub Check: Security Scan
- GitHub Check: Backend Tests
🔇 Additional comments (23)
src/components/ui/completion-dialog.tsx (1)
77-78: Good improvement to parameter naming clarity.The change from
successfulandfailedtocompletedandtotalmakes the parameters more semantic and user-friendly. This aligns well with the broader localization string simplifications mentioned in the PR summary.src/__tests__/services/mod-translation-check.test.ts (1)
1-214: Excellent comprehensive test coverage for mod translation detection.The test suite covers all essential scenarios:
- Basic translation existence checking for different formats
- Case sensitivity handling for language codes
- Error handling and graceful degradation
- Frontend integration simulation with realistic mod data
- Edge cases including empty language codes and special characters
- Concurrent translation checks
The test structure is well-organized and the mocking approach effectively isolates the frontend service layer.
src/__tests__/integration/mod-translation-flow.test.ts (3)
1-16: Comprehensive integration test setup.The test structure properly mocks the FileService and provides clean setup/teardown. The integration approach effectively tests the complete flow from mod scanning to translation detection.
17-128: Excellent end-to-end flow testing.The test comprehensively covers the complete mod translation detection workflow:
- Mod file discovery and analysis
- Translation existence checking
- Proper result aggregation
- Realistic mod data scenarios
This provides valuable coverage for the integration between frontend and backend services.
129-278: Excellent coverage of edge cases and performance scenarios.The tests cover:
- Different target languages with proper expectations
- Configuration handling for skipping existing translations
- Error handling throughout the flow
- Concurrent execution performance testing
The performance test is particularly valuable for ensuring the translation checks can handle multiple mods efficiently.
src/components/debug/translation-check-debug.tsx (3)
1-51: Excellent debugging utility component.This component provides a comprehensive debugging interface for mod translation detection with:
- Clear form inputs for mod path, ID, and target language
- Proper loading states and error handling
- Conditional debug command execution (development only)
- Good user feedback with formatted results
The implementation properly separates regular functionality from debug features.
53-71: Smart file selection and mod ID extraction.The file browsing functionality is well-implemented:
- Uses appropriate file dialog filters for JAR files
- Automatically attempts to extract mod ID from filename using regex
- Handles edge cases gracefully with fallbacks
The regex pattern
^(.+?)-\d+effectively extracts the mod name portion before version numbers.
73-123: Clean and accessible UI implementation.The component provides a user-friendly interface with:
- Proper form labeling for accessibility
- Responsive layout with appropriate spacing
- Clear visual feedback for loading states
- Preformatted output for debug information
- Consistent styling using the design system
src-tauri/src/minecraft/debug_translation_check.rs (2)
1-54: Comprehensive debug function for translation detection.The
debug_check_translationfunction provides excellent debugging capabilities:
- Tests multiple language codes including case variations (ja_jp, JA_JP, ja_JP)
- Comprehensive language coverage for common locales
- Lists all language files in the JAR archive
- Proper error handling and informative output
- Reuses the existing
check_mod_translation_existsfunction for consistencyThe implementation helps diagnose translation detection issues effectively.
56-124: Well-designed Tauri command for frontend integration.The
debug_mod_translation_checkcommand provides:
- String-based output suitable for frontend display
- Validation of expected file patterns (
assets/{mod_id}/lang/)- Clear visual indicators (✓/✗) for pattern matching
- Comprehensive error handling with descriptive messages
- Proper handling of ZIP archive reading failures
The pattern validation is particularly useful for diagnosing why translations might not be detected.
src/components/tabs/mods-tab.tsx (6)
139-162: Well-implemented translation existence checkThe implementation correctly:
- Only checks when target language is provided and skip setting is enabled
- Handles errors gracefully without interrupting the scan
- Stores the result for UI display
236-241: Good optimization: Resource pack creation deferredMoving resource pack creation after skip checking prevents unnecessary directory creation when all mods are already translated.
309-332: Excellent early exit optimizationThe early exit when all mods are skipped improves user experience by:
- Avoiding unnecessary processing
- Providing clear feedback about why no translations occurred
- Properly cleaning up state
340-342: Fixed progress calculationUsing
jobs.lengthinstead ofsortedTargets.lengthcorrectly ensures progress reaches 100% by only counting actual translation jobs, not skipped items. This addresses the PR's main objective.
464-479: Clean UI implementation for translation statusThe new column effectively displays translation existence status with:
- Clear visual indicators (green for exists, gray for new)
- Proper conditional rendering
- Accessible color choices for both light and dark themes
353-355: Session path handling is correctThe
TranslationTabwrapper in src/components/tabs/common/translation-tab.tsx (invoke at line ~382) callscreate_logs_directory_with_session, so the directory${selectedDirectory}/logs/localizer/${sessionId}is guaranteed to exist before the ModsTab runs. No additional path-creation call is needed.
create_logs_directory_with_sessionis invoked once in the common translation component.- ModsTab correctly uses the provided
sessionIdand manual string construction to reference the existing directory.No changes required.
src-tauri/src/minecraft/mod_translation_test.rs (4)
7-72: Well-designed test helper functionThe
create_mock_mod_jarhelper is excellently structured:
- Creates realistic mod JARs with proper metadata
- Supports multiple language formats
- Always ensures en_us.json exists as source
- Returns TempDir for automatic cleanup
110-135: Excellent case sensitivity testingThe case insensitivity test is crucial for robust translation detection, covering both fully uppercase and mixed case scenarios.
203-272: Comprehensive realistic mod structure testThis test effectively simulates real-world mod structures with:
- Proper META-INF directory
- Multiple language files
- Additional assets (textures)
- Systematic verification of multiple languages
393-439: Excellent concurrent access testingThe concurrent access test thoroughly verifies thread safety by:
- Running multiple parallel checks on the same file
- Testing both existing and non-existing languages
- Properly awaiting all async operations
- Clear assertions for each language result
src/components/tabs/common/translation-tab.tsx (3)
94-103: Well-designed interface updatesThe interface changes properly support:
- Optional target language for pre-checking during scans
- Required session ID for consistent logging across translation workflow
180-189: Good simplification of directory handlingRemoving the NATIVE_DIALOG prefix handling simplifies the code and moves this concern to the appropriate layer (likely the backend).
Also applies to: 213-214, 367-368, 502-502
370-399: Excellent session management implementationThe centralized session ID generation and directory creation:
- Ensures consistent session handling across all tabs
- Properly orders operations (clear logs before new session)
- Handles errors gracefully without blocking translation
- Provides good debugging information
| // Test data generators for debugging | ||
| export const generateTestModData = () => { | ||
| return [ | ||
| { | ||
| id: 'silentgear', | ||
| name: 'Silent Gear', | ||
| path: '/mods/SilentGear-1.19.2-3.2.2.jar', | ||
| expectedTranslations: { | ||
| ja_jp: true, | ||
| zh_cn: true, | ||
| ko_kr: false, | ||
| de_de: false, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'create', | ||
| name: 'Create', | ||
| path: '/mods/create-1.19.2-0.5.1.jar', | ||
| expectedTranslations: { | ||
| ja_jp: true, | ||
| zh_cn: true, | ||
| ko_kr: true, | ||
| de_de: true, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'custommod', | ||
| name: 'Custom Mod', | ||
| path: '/mods/custommod-1.0.0.jar', | ||
| expectedTranslations: { | ||
| ja_jp: false, | ||
| zh_cn: false, | ||
| ko_kr: false, | ||
| de_de: false, | ||
| }, | ||
| }, | ||
| ]; | ||
| }; No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move test data generator to a separate utility file.
The generateTestModData function should not be exported from a test file. This creates an anti-pattern where test files become dependencies for other modules.
Move this function to a dedicated test utility file:
-// Test data generators for debugging
-export const generateTestModData = () => {Create a new file src/__tests__/test-utils/mod-test-data.ts:
export const generateTestModData = () => {
return [
{
id: 'silentgear',
name: 'Silent Gear',
path: '/mods/SilentGear-1.19.2-3.2.2.jar',
expectedTranslations: {
ja_jp: true,
zh_cn: true,
ko_kr: false,
de_de: false,
},
},
// ... rest of the data
];
};Then import it in both test files that need it.
🧰 Tools
🪛 Biome (1.9.4)
[error] 215-252: Do not export from a test file.
(lint/suspicious/noExportsInTest)
🤖 Prompt for AI Agents
In src/__tests__/services/mod-translation-check.test.ts around lines 215 to 252,
the generateTestModData function is exported from the test file, which creates
an anti-pattern by making test files dependencies for other modules. To fix
this, move the generateTestModData function to a new utility file at
src/__tests__/test-utils/mod-test-data.ts, then import this function into any
test files that require it instead of exporting it from the test file.
| case 'check_mod_translation_exists': | ||
| // Use test data to determine if translation exists | ||
| const testData = generateTestModData(); | ||
| const mod = testData.find(m => m.id === args.modId); | ||
| if (mod && mod.expectedTranslations[args.targetLanguage as keyof typeof mod.expectedTranslations] !== undefined) { | ||
| return Promise.resolve(mod.expectedTranslations[args.targetLanguage as keyof typeof mod.expectedTranslations]); | ||
| } | ||
| return Promise.resolve(false); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix switch declaration scope issue.
The testData and mod variables declared in the switch case can be accessed from other cases, which could lead to unexpected behavior.
Wrap the declarations in a block:
case 'check_mod_translation_exists':
- // Use test data to determine if translation exists
- const testData = generateTestModData();
- const mod = testData.find(m => m.id === args.modId);
- if (mod && mod.expectedTranslations[args.targetLanguage as keyof typeof mod.expectedTranslations] !== undefined) {
- return Promise.resolve(mod.expectedTranslations[args.targetLanguage as keyof typeof mod.expectedTranslations]);
- }
- return Promise.resolve(false);
+ {
+ // Use test data to determine if translation exists
+ const testData = generateTestModData();
+ const mod = testData.find(m => m.id === args.modId);
+ if (mod && mod.expectedTranslations[args.targetLanguage as keyof typeof mod.expectedTranslations] !== undefined) {
+ return Promise.resolve(mod.expectedTranslations[args.targetLanguage as keyof typeof mod.expectedTranslations]);
+ }
+ return Promise.resolve(false);
+ }📝 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.
| case 'check_mod_translation_exists': | |
| // Use test data to determine if translation exists | |
| const testData = generateTestModData(); | |
| const mod = testData.find(m => m.id === args.modId); | |
| if (mod && mod.expectedTranslations[args.targetLanguage as keyof typeof mod.expectedTranslations] !== undefined) { | |
| return Promise.resolve(mod.expectedTranslations[args.targetLanguage as keyof typeof mod.expectedTranslations]); | |
| } | |
| return Promise.resolve(false); | |
| case 'check_mod_translation_exists': | |
| { | |
| // Use test data to determine if translation exists | |
| const testData = generateTestModData(); | |
| const mod = testData.find(m => m.id === args.modId); | |
| if ( | |
| mod && | |
| mod.expectedTranslations[ | |
| args.targetLanguage as keyof typeof mod.expectedTranslations | |
| ] !== undefined | |
| ) { | |
| return Promise.resolve( | |
| mod.expectedTranslations[ | |
| args.targetLanguage as keyof typeof mod.expectedTranslations | |
| ] | |
| ); | |
| } | |
| return Promise.resolve(false); | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 77-77: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 78-78: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In src/__tests__/integration/mod-translation-flow.test.ts around lines 75 to 82,
the variables testData and mod are declared directly inside a switch case, which
can cause scope leakage to other cases. To fix this, wrap the code inside the
'check_mod_translation_exists' case in curly braces to create a block scope,
ensuring testData and mod are scoped only within this case and preventing
unintended access from other cases.
| #[test] | ||
| fn test_mod_translation_detection_integration() { | ||
| // Create temp directory structure like Minecraft | ||
| let temp_dir = TempDir::new().unwrap(); | ||
| let minecraft_dir = temp_dir.path(); | ||
| let mods_dir = minecraft_dir.join("mods"); | ||
| fs::create_dir_all(&mods_dir).unwrap(); | ||
|
|
||
| // Test Case 1: Mod with Japanese translation (JSON format) | ||
| let mod1_translations = vec![ | ||
| ("en_us", "json", r#"{"item.testmod1.item": "Test Item"}"#), | ||
| ( | ||
| "ja_jp", | ||
| "json", | ||
| r#"{"item.testmod1.item": "テストアイテム"}"#, | ||
| ), | ||
| ]; | ||
| let mod1_path = create_test_mod_jar(&mods_dir, "testmod1", "Test Mod 1", mod1_translations); | ||
|
|
||
| // Test Case 2: Mod with legacy .lang format | ||
| let mod2_translations = vec![ | ||
| ("en_us", "lang", "item.testmod2.item=Test Item 2"), | ||
| ("ja_jp", "lang", "item.testmod2.item=テストアイテム2"), | ||
| ]; | ||
| let mod2_path = create_test_mod_jar(&mods_dir, "testmod2", "Test Mod 2", mod2_translations); | ||
|
|
||
| // Test Case 3: Mod without Japanese translation | ||
| let mod3_translations = vec![ | ||
| ("en_us", "json", r#"{"item.testmod3.item": "Test Item 3"}"#), | ||
| ( | ||
| "de_de", | ||
| "json", | ||
| r#"{"item.testmod3.item": "Test Artikel 3"}"#, | ||
| ), | ||
| ]; | ||
| let mod3_path = create_test_mod_jar(&mods_dir, "testmod3", "Test Mod 3", mod3_translations); | ||
|
|
||
| // Test Case 4: Mod with mixed case language codes | ||
| let mod4_translations = vec![ | ||
| ("en_us", "json", r#"{"item.testmod4.item": "Test Item 4"}"#), | ||
| ( | ||
| "JA_JP", | ||
| "json", | ||
| r#"{"item.testmod4.item": "テストアイテム4"}"#, | ||
| ), // Upper case | ||
| ]; | ||
| let mod4_path = create_test_mod_jar(&mods_dir, "testmod4", "Test Mod 4", mod4_translations); | ||
|
|
||
| // Print paths for manual testing | ||
| println!("Created test mods:"); | ||
| println!(" - Mod 1 (with ja_jp.json): {:?}", mod1_path); | ||
| println!(" - Mod 2 (with ja_jp.lang): {:?}", mod2_path); | ||
| println!(" - Mod 3 (without ja_jp): {:?}", mod3_path); | ||
| println!(" - Mod 4 (with JA_JP.json): {:?}", mod4_path); | ||
|
|
||
| // Verify files exist | ||
| assert!(mod1_path.exists(), "Mod 1 JAR should exist"); | ||
| assert!(mod2_path.exists(), "Mod 2 JAR should exist"); | ||
| assert!(mod3_path.exists(), "Mod 3 JAR should exist"); | ||
| assert!(mod4_path.exists(), "Mod 4 JAR should exist"); | ||
|
|
||
| // Additional test: Create a mod with complex structure | ||
| let mod5_path = mods_dir.join("complexmod-1.0.0.jar"); | ||
| let file = File::create(&mod5_path).unwrap(); | ||
| let mut zip = ZipWriter::new(file); | ||
| let options = FileOptions::default().compression_method(zip::CompressionMethod::Deflated); | ||
|
|
||
| // Add various files that might confuse the detection | ||
| zip.start_file("assets/complexmod/lang/en_us.json", options) | ||
| .unwrap(); | ||
| zip.write_all(br#"{"item.complexmod.item": "Complex Item"}"#) | ||
| .unwrap(); | ||
|
|
||
| // Add file with similar name but wrong path | ||
| zip.start_file("assets/wrongmod/lang/ja_jp.json", options) | ||
| .unwrap(); | ||
| zip.write_all(br#"{"item.wrongmod.item": "Wrong Item"}"#) | ||
| .unwrap(); | ||
|
|
||
| // Add correct Japanese translation | ||
| zip.start_file("assets/complexmod/lang/ja_jp.json", options) | ||
| .unwrap(); | ||
| zip.write_all(r#"{"item.complexmod.item": "複雑なアイテム"}"#.as_bytes()) | ||
| .unwrap(); | ||
|
|
||
| // Add other assets that shouldn't affect detection | ||
| zip.start_file("assets/complexmod/textures/item/test.png", options) | ||
| .unwrap(); | ||
| zip.write_all(b"FAKE_PNG_DATA").unwrap(); | ||
|
|
||
| zip.start_file("data/complexmod/recipes/test.json", options) | ||
| .unwrap(); | ||
| zip.write_all(br#"{"type": "minecraft:crafting_shaped"}"#) | ||
| .unwrap(); | ||
|
|
||
| zip.finish().unwrap(); | ||
|
|
||
| println!(" - Mod 5 (complex structure): {:?}", mod5_path); | ||
| assert!(mod5_path.exists(), "Mod 5 JAR should exist"); | ||
|
|
||
| // The actual check_mod_translation_exists calls would be made from the application | ||
| println!("\nTest mods created successfully in: {:?}", mods_dir); | ||
| println!("\nExpected results when checking for ja_jp translations:"); | ||
| println!(" - testmod1: Should find translation (ja_jp.json exists)"); | ||
| println!(" - testmod2: Should find translation (ja_jp.lang exists)"); | ||
| println!(" - testmod3: Should NOT find translation (only de_de exists)"); | ||
| println!(" - testmod4: Case sensitivity test - depends on implementation"); | ||
| println!(" - complexmod: Should find translation (correct path exists)"); | ||
| } |
There was a problem hiding this comment.
Test doesn't perform actual integration testing
This test creates mod JAR files but doesn't actually call check_mod_translation_exists to verify the translation detection functionality. It only prints expected results without any assertions.
Consider either:
- Renaming this to reflect it's a test fixture generator
- Adding actual calls to
check_mod_translation_existswith proper assertions
🤖 Prompt for AI Agents
In src-tauri/tests/mod_translation_integration.rs between lines 66 and 174, the
test creates mod JAR files and prints expected results but does not call the
function check_mod_translation_exists to verify translation detection. To fix
this, add calls to check_mod_translation_exists for each created mod JAR and
include assertions that confirm whether the function correctly detects the
presence or absence of Japanese translations as expected. This will convert the
test into a true integration test validating the translation detection logic.
| #[test] | ||
| fn test_edge_cases() { | ||
| let temp_dir = TempDir::new().unwrap(); | ||
| let mods_dir = temp_dir.path().join("mods"); | ||
| fs::create_dir_all(&mods_dir).unwrap(); | ||
|
|
||
| // Edge Case 1: Empty language file | ||
| let edge1_path = mods_dir.join("emptymod-1.0.0.jar"); | ||
| let file = File::create(&edge1_path).unwrap(); | ||
| let mut zip = ZipWriter::new(file); | ||
| let options = FileOptions::default(); | ||
|
|
||
| zip.start_file("assets/emptymod/lang/ja_jp.json", options) | ||
| .unwrap(); | ||
| zip.write_all(b"{}").unwrap(); // Empty JSON | ||
|
|
||
| zip.finish().unwrap(); | ||
|
|
||
| // Edge Case 2: Malformed path separators | ||
| let edge2_path = mods_dir.join("pathmod-1.0.0.jar"); | ||
| let file = File::create(&edge2_path).unwrap(); | ||
| let mut zip = ZipWriter::new(file); | ||
|
|
||
| // Using backslashes (Windows-style) | ||
| zip.start_file(r"assets\pathmod\lang\ja_jp.json", options) | ||
| .unwrap(); | ||
| zip.write_all(br#"{"test": "test"}"#).unwrap(); | ||
|
|
||
| zip.finish().unwrap(); | ||
|
|
||
| // Edge Case 3: Multiple language files in different locations | ||
| let edge3_path = mods_dir.join("multimod-1.0.0.jar"); | ||
| let file = File::create(&edge3_path).unwrap(); | ||
| let mut zip = ZipWriter::new(file); | ||
|
|
||
| // Correct location | ||
| zip.start_file("assets/multimod/lang/ja_jp.json", options) | ||
| .unwrap(); | ||
| zip.write_all(br#"{"correct": "true"}"#).unwrap(); | ||
|
|
||
| // Wrong location (should be ignored) | ||
| zip.start_file("lang/ja_jp.json", options).unwrap(); | ||
| zip.write_all(br#"{"wrong": "true"}"#).unwrap(); | ||
|
|
||
| zip.finish().unwrap(); | ||
|
|
||
| println!("\nEdge case test mods created:"); | ||
| println!(" - Empty language file: {:?}", edge1_path); | ||
| println!(" - Path separator test: {:?}", edge2_path); | ||
| println!(" - Multiple locations: {:?}", edge3_path); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Edge case test doesn't verify actual behavior
Similar to the main test, this function only creates test files without verifying the actual behavior of check_mod_translation_exists.
Add actual test assertions:
// After creating edge1_path
let result = check_mod_translation_exists(edge1_path.to_str().unwrap(), "emptymod", "ja_jp").await;
assert!(result.is_ok() && result.unwrap(), "Empty JSON file should still be detected");
// After creating edge2_path
let result = check_mod_translation_exists(edge2_path.to_str().unwrap(), "pathmod", "ja_jp").await;
// Test the actual behavior with incorrect path separators
// After creating edge3_path
let result = check_mod_translation_exists(edge3_path.to_str().unwrap(), "multimod", "ja_jp").await;
assert!(result.is_ok() && result.unwrap(), "Should find translation in correct location");🤖 Prompt for AI Agents
In src-tauri/tests/mod_translation_integration.rs between lines 176 and 226, the
test_edge_cases function currently only creates test mod files but does not
verify the behavior of check_mod_translation_exists. To fix this, add
asynchronous calls to check_mod_translation_exists after creating each test mod
file (edge1_path, edge2_path, edge3_path) and assert the expected outcomes. For
edge1_path, assert that the function returns Ok(true) despite the empty JSON;
for edge2_path, test the behavior with the malformed path separators and assert
accordingly; for edge3_path, assert that the translation is found in the correct
location. This will ensure the test actually verifies the functionality.
| zip.start_file(r"assets\pathmod\lang\ja_jp.json", options) | ||
| .unwrap(); |
There was a problem hiding this comment.
Incorrect path separator in ZIP entry
ZIP format specification requires forward slashes as path separators, even on Windows. Using backslashes may cause issues with ZIP readers.
- zip.start_file(r"assets\pathmod\lang\ja_jp.json", options)
+ // Test with forward slashes (ZIP standard)
+ zip.start_file("assets/pathmod/lang/ja_jp.json", options)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src-tauri/tests/mod_translation_integration.rs around lines 200 to 201, the
ZIP entry path uses backslashes which are not compliant with the ZIP format
specification. Replace the backslashes in the path string with forward slashes
to ensure compatibility across different ZIP readers and platforms.
- Update all format! strings to use inline format arguments - Fix debug_translation_check.rs format strings - Fix mod_translation_test.rs format strings - Ensures CI/CD Clippy validation passes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add profileDirectory validation in translation-runner.ts before invoking - Add user notification for batch summary update failures - Fix optional chaining in unified-log-viewer.tsx - Replace println! with debug! macro in backup.rs for proper logging - Add comment explaining UTF-8 string handling in integration tests These improvements enhance error handling, logging consistency, and code quality. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add useAppStore mock to provide profileDirectory in tests - Update expected minecraftDir values from empty string to '/test/modpack' - Ensures all 161 Jest tests pass after Code Rabbit improvements 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Applied rustfmt to fix formatting in: - src-tauri/src/minecraft/mod_translation_test.rs (line 46, 419) - Shortened long format strings to single lines 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src-tauri/src/minecraft/mod_translation_test.rs (1)
7-66: Well-designed helper function with room for minor improvements.The
create_mock_mod_jarfunction effectively creates realistic test JAR files. Consider these enhancements:
- The function could be made more flexible by allowing custom mod metadata
- Error handling could be more specific about which operation failed
Consider this enhancement to make the function more flexible:
fn create_mock_mod_jar( mod_id: &str, languages: Vec<(&str, &str)>, // (language_code, format) e.g., ("ja_jp", "json") + custom_metadata: Option<&str>, // Optional custom fabric.mod.json content ) -> Result<TempDir, Box<dyn std::error::Error>> { // ... existing code ... - let mod_json = format!( - r#"{{ - "schemaVersion": 1, - "id": "{mod_id}", - "version": "1.0.0", - "name": "Test Mod" - }}"# - ); + let mod_json = custom_metadata.unwrap_or(&format!( + r#"{{ + "schemaVersion": 1, + "id": "{mod_id}", + "version": "1.0.0", + "name": "Test Mod" + }}"# + ));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src-tauri/src/minecraft/mod_translation_test.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build - aarch64-apple-darwin
- GitHub Check: Build - x86_64-pc-windows-msvc
- GitHub Check: Build - x86_64-unknown-linux-gnu
- GitHub Check: Build - x86_64-apple-darwin
- GitHub Check: Security Scan
- GitHub Check: Backend Tests
🔇 Additional comments (7)
src-tauri/src/minecraft/mod_translation_test.rs (7)
68-152: Excellent basic test coverage with clear structure.The basic functionality tests comprehensively cover the core scenarios:
- JSON and legacy lang format support
- Case-insensitive language code matching
- Mixed format handling within the same mod
Each test follows a consistent pattern with clear assertions and descriptive error messages.
154-195: Comprehensive error handling test coverage.The error tests effectively validate the function's resilience:
- Wrong mod ID verification ensures path-based lookup works correctly
- Invalid JAR handling tests ZIP file validation
- Nonexistent file tests ensure proper file system error propagation
197-265: Excellent realistic scenario testing.This test effectively validates the function against real-world mod structures:
- Comprehensive mod layout with META-INF, assets, and multiple language files
- Realistic translation content in different languages
- Multiple test cases in a single test for efficiency
- Uses appropriate compression method for realistic file sizes
267-298: Good edge case coverage for input validation.These tests effectively validate boundary conditions:
- Special characters in mod IDs (common in real mods with hyphens and underscores)
- Empty language code handling for input validation
300-342: Excellent performance validation test.This test effectively validates performance characteristics:
- Simulates large mods with 2000 files (realistic for complex mods)
- Strategic placement of target file to test scanning efficiency
- Reasonable 1-second performance expectation
- Proper timing measurement and assertion
344-383: Well-designed test for complex mod structures.This test effectively validates handling of mods with nested JARs:
- Realistic scenario for mod loaders that include dependency JARs
- Proper nested ZIP creation using in-memory cursor
- Ensures the function correctly identifies the main mod's language files
385-428: Excellent concurrency and thread safety validation.This test comprehensively validates concurrent access:
- True async concurrency using
tokio::spawnfor multiple simultaneous checks- Tests both existing and non-existing languages for comprehensive validation
- Proper result validation with expected outcomes using pattern matching
- Ensures thread safety of the underlying ZIP file reading operations
…n_integration test
Updated all format! macros to use inline format arguments:
- Fixed 13 occurrences of format string warnings
- Changed format!("{}-1.0.0.jar", mod_id) to format!("{mod_id}-1.0.0.jar")
- Updated println! statements to use inline format syntax
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <[email protected]>
Summary
Changes Made
🔧 Progress Calculation Fixes
sortedTargets.lengthtojobs.lengthto account for skipped translations🎨 UI Improvements
🧹 Code Quality
rawLogContent,sortedTargetsscope issues)cargo fmt)Technical Details
Progress Calculation Logic
UI Responsiveness
Test Results
Files Modified
mods-tab.tsx,quests-tab.tsx,guidebooks-tab.tsx,custom-files-tab.tsxtranslation-history-dialog.tsx,unified-log-viewer.tsxbackup.rs,filesystem.rs,lib.rs,minecraft/mod.rs🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Localization
Refactor