feat: implement file count-based progress bars and remove path settings #21
Conversation
Implement CodeRabbit's suggestion to wrap the sorting logic in useMemo to prevent unnecessary recalculations on every render. This improves performance when dealing with large datasets by only recalculating when sessions or sortConfig change. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…ions Add real-time progress feedback during file scanning operations showing current file being processed and total count. Updates every 1 file or 1000ms (whichever comes first) following the "Simple over Easy" design. - Add scan progress event emission in Rust backend with 1000ms throttling - Add scan progress state management to Zustand store - Update all tab components to display scan progress during operations - Show "Scanning: filename (X files)" instead of generic "Scanning..."
Improve scan progress reporting to address user feedback about poor visibility: - Show progress as "X / Y files (Z%)" instead of just "(X files)" - Count total files before scanning for accurate progress percentage - Update progress more frequently (every 10 files OR every 200ms) - Emit progress events when finding relevant files (JAR, SNBT, JSON) - Apply improvements to all scan types (mods, quests, guidebooks, custom files) This provides better user feedback during scan operations with clear progress indication and reduces the time when no progress is shown.
Fix the issue where scan progress was only shown during initial file discovery but not during the time-consuming file analysis phase. - Add progress updates during JAR analysis in mods-tab.tsx - Add progress updates during quest file analysis in quests-tab.tsx - Add progress updates during Patchouli book extraction in guidebooks-tab.tsx - Add progress updates during custom file processing in custom-files-tab.tsx - Add resetScanProgress() calls in all finally blocks This ensures users see continuous progress updates throughout the entire scan process, not just "Please wait..." during the analysis phase.
Add a compact progress bar to complement the text-based progress display during scan operations. The progress bar shows visual completion percentage alongside the existing "X / Y files (Z%)" text display. Features: - Small, compact progress bar (height: 1.5rem) - Only shows when both processedCount and totalCount are available - Centered with max-width for better visual balance - Maintains the existing text progress display above it - Consistent with the "Simple over Easy" design philosophy This provides better visual feedback while keeping the interface clean and uncluttered.
Fix layout issues with the scan progress display: - Set fixed width (w-80) for progress bar to prevent stretching/shrinking - Change file name alignment from center to left (text-left) - Keep progress statistics centered (text-center) - Remove text-center from main container to allow individual control This provides consistent visual layout regardless of file name length and better readability with left-aligned file names.
Fix the scan progress container from expanding/contracting based on filename length: - Set fixed width (w-96) for the progress content container - Add truncate class to filename text to handle overflow - Keep progress bar at fixed width (w-80) within the container This ensures consistent layout regardless of filename length and prevents the progress bar from appearing to stretch or shrink.
Fix the delay between clicking scan button and progress display appearing by initializing scan progress state immediately when scanning begins. Changes: - Set initial scan progress state right after setScanning(true) - Initialize with empty currentFile, 0 processedCount, undefined totalCount - Set appropriate scanType for each tab (mods, quests, guidebooks, custom-files) - Apply fix to all tab components consistently This provides immediate visual feedback that scanning has started, instead of waiting for the first file to be discovered/processed by the backend.
Improve scan button responsiveness especially during re-scans by: - Move heavy state clearing operations to async setTimeout to avoid blocking UI - Show "Initializing scan..." message immediately for better user feedback - Reduce UI blocking during translation targets clearing - Apply optimizations to all tab components consistently This reduces the perceived delay when clicking scan button after previous scans by preventing React re-render blocking during large state updates.
Revert problematic async setTimeout and implement more effective optimizations: - Remove setTimeout that caused additional delay - Add conditional state updates to avoid unnecessary operations - Use useCallback to memoize handleScan function - Import useMemo and useCallback for future optimizations - Only clear state if it actually has content This provides immediate response while avoiding unnecessary React re-renders and state updates during re-scan operations.
Revert useCallback and conditional state updates that were causing delays: - Remove useCallback with problematic dependencies (onScan changes every render) - Remove conditional state clearing that adds unnecessary checks - Return to simple, direct state updates for better performance - Keep only essential optimization: check translationResults length before clearing The original implementation was actually faster. The perceived delay was not from React re-renders but from the actual scan processing time.
…hilosophy Remove unnecessary optimizations and return to straightforward implementation: - Remove useCallback and complex dependency management - Remove conditional state clearing (if checks) - Remove unused imports (useCallback, useMemo) - Use direct state updates for clarity and simplicity - Keep only essential functionality This follows the "EasyよりもSimple" design principle - prefer simple, understandable code over complex optimizations that don't provide clear benefits.
Fix the delay between scan start and progress display by adding intermediate progress updates immediately after file discovery phase: - Show "Analyzing [file type]..." after file discovery completes - Display total file count before detailed analysis begins - Apply to all tab types (mods, quests, guidebooks, custom files) - Provide immediate visual feedback to users This addresses the two-phase scan process: 1. File discovery (fast) - now shows immediate progress 2. File analysis (slow) - already had progress updates Users now see continuous progress feedback throughout the entire scan process.
Fix the issue where scan button remains in pressed state due to UI blocking: - Move heavy state clearing operations to requestAnimationFrame - Allow UI to update with isScanning=true before clearing large datasets - Extract directory path before clearing to avoid delays - Prevent synchronous React re-renders from blocking button animations This ensures the scan button animation and progress display appear immediately without being blocked by clearing existing translation targets.
- Add click feedback and hover states to all interactive elements - Implement smooth transitions for buttons, inputs, and form controls - Add loading animations with spinners for scan and translate operations - Enhance dialog and modal animations with backdrop blur - Add staggered animations for table rows and data loading - Create consistent animation timing (200-300ms) across components - Improve loading states with enhanced visual feedback - Add error display animations for better user awareness
…essages - Add check functions for existing translations in mods, quests, and guidebooks - Implement skipExistingTranslations functionality to avoid duplicate work - Improve error messages with i18n translation keys for better localization - Add validation for mod, quest, and guidebook translation files - Enhance filesystem error handling with structured error messages
- Add PathsConfig interface with directory path properties - Include paths property in AppConfig interface - Add PATH_DEFAULTS constants for default path values - Fix TypeScript compilation errors related to missing paths property
Backend Changes: - Add scan progress event emission in filesystem.rs with file count tracking - Add translation existence check functions for mods, quests, and guidebooks - Add read_session_log function for session log retrieval - Add test features and dev dependencies to Cargo.toml - Export new command functions in lib.rs Frontend Changes: - Implement scan progress UI with progress bars and file count display - Add scan state management and progress tracking - Update tab switching to prevent switching during scan operations - Add loading animations and improved UI feedback - Update various UI components with progress-related improvements Features: - Real-time progress updates during file scanning - File count-based progress indicators - Responsive UI with loading states - Tab locking during scan operations - Enhanced error handling and user feedback
- Replace serde_json::to_string_pretty with serialize_json_sorted in backup.rs - Update config.rs to use sorted JSON serialization for all config operations - Add session ID format validation in backup.rs - Update lang_file_test.rs to test sorted JSON functionality - Ensure consistent JSON key ordering across all output files - Improve file diff readability and maintainability
…rovements - Add session ID validation with proper format checking - Implement session log reading functionality for debugging - Add JSON sorting for consistent backup metadata and config output - Improve backup session management with better validation - Enhance logging system with session-specific log retrieval
English translations: - Add skipExistingTranslations setting translations - Add scanInProgress error message - Add detailed error messages for directory not found scenarios - Add session log related translations Japanese translations: - Add corresponding Japanese translations for all new English strings - Maintain consistency with existing Japanese translation style - Add proper Japanese translations for scan progress features Features: - Support for skip existing translations setting - Enhanced error message localization - Session log viewing translations - Improved user experience with localized messages
- Remove unused FileService import that was causing ESLint error - Fix linting issue after applying cargo fmt to Rust files - Ensure all PR validation checks pass
WalkthroughThis update introduces scan progress tracking and skipping of already translated files across mods, quests, and guidebooks, with corresponding UI and backend enhancements. It adds new backend commands for translation existence checks, implements cross-platform path utilities, and refines error reporting and UI responsiveness. Numerous UI components receive responsive and interactive style improvements, and session log viewing is integrated into the translation history dialog. JSON serialization is standardized for sorted key output, and configuration defaults and structure are updated to support new features. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant Store
participant Backend
participant Filesystem
UI->>Store: Set isScanning = true
UI->>Backend: Start scan (e.g., get_mod_files)
Backend->>Filesystem: Scan directory, count files
loop For each file (periodically)
Filesystem->>Backend: Emit scan_progress event (current file, counts)
Backend->>UI: scan_progress event
UI->>Store: Update scanProgress state
end
Filesystem->>Backend: Complete scan, emit final scan_progress
Backend->>UI: scan_progress (completed)
UI->>Store: Reset scanProgress, set isScanning = false
sequenceDiagram
participant UI
participant Backend
UI->>Backend: check_mod_translation_exists (mod_path, mod_id, lang)
Backend->>Filesystem: Open mod archive, check for lang files
Filesystem-->>Backend: Exists? (true/false)
Backend-->>UI: Return true/false
UI->>Backend: check_quest_translation_exists (quest_path, lang)
Backend->>Filesystem: Check for quest translation files
Filesystem-->>Backend: Exists? (true/false)
Backend-->>UI: Return true/false
UI->>Backend: check_guidebook_translation_exists (guidebook_path, mod_id, book_id, lang)
Backend->>Filesystem: Open guidebook archive, check for translation file
Filesystem-->>Backend: Exists? (true/false)
Backend-->>UI: Return true/false
sequenceDiagram
participant UI
participant Backend
UI->>Backend: read_session_log (minecraft_dir, session_id)
Backend->>Filesystem: Read log file for session
alt File exists
Filesystem-->>Backend: Return log content
Backend-->>UI: Return log content
else File missing/error
Filesystem-->>Backend: Return error
Backend-->>UI: Return error
end
Possibly related PRs
Poem
✨ 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: 2
🔭 Outside diff range comments (2)
src-tauri/src/filesystem.rs (2)
189-255: Add progress tracking for KubeJS lang file scanning.The KubeJS lang file scanning section doesn't emit progress events, which creates an inconsistent user experience compared to SNBT file scanning.
Add progress tracking similar to the SNBT scanning section. You'll need to count files first and emit progress events during the scan.
381-381: Use consistent error message format.This error message doesn't follow the new prefixed format used elsewhere in the file.
- return Err("No FTB quests directory found. Checked: config/ftbquests/quests/, config/ftbquests/normal/, and config/ftbquests/".to_string()); + return Err("errors.questsDirectoryNotFound:::config/ftbquests/quests/, config/ftbquests/normal/, config/ftbquests/".to_string());
♻️ Duplicate comments (2)
src/components/tabs/guidebooks-tab.tsx (1)
220-220: Consistency: Default value forskipExistingTranslations.Same as in
mods-tab.tsx, this defaults totrue. For consistency across all tabs, consider using the same default value approach.src/components/tabs/quests-tab.tsx (1)
272-272: Consistency: Default value forskipExistingTranslations.Same default
truevalue as other tabs. Consider standardizing this across all components.
🧹 Nitpick comments (10)
src/lib/utils/path-utils.ts (1)
42-72: Consider edge case handling in relative path calculation.The function handles most cases well, but the fallback logic (lines 64-71) that returns the last 2 segments when paths don't match might be confusing. Consider documenting this behavior more clearly or providing a more predictable fallback.
// If not a child of base path, return last 2 segments as fallback + // This provides a meaningful relative representation even when paths don't share a common base const parts = fullPath.split(/[/\\]/); if (parts.length >= 2) { return parts.slice(-2).join('/'); }src/components/tabs/mods-tab.tsx (2)
49-91: Consider adding error handling for the event listener cleanup.The current implementation catches errors during listener setup but returns an empty function. Consider logging the error or providing user feedback when the scan progress listener fails to initialize.
} catch (error) { console.error('Failed to set up scan progress listener:', error); - return () => {}; + // Return a no-op function but ensure we don't lose the error context + return () => { + console.warn('Scan progress listener was not properly initialized'); + }; }
233-233: Consider the default value forskipExistingTranslations.The nullish coalescing operator defaults to
true, which means translations will be skipped by default even if the user hasn't configured this setting. This might be unexpected behavior for users.Consider defaulting to
falseto ensure all items are translated unless explicitly configured otherwise:-if (config.translation.skipExistingTranslations ?? true) { +if (config.translation.skipExistingTranslations ?? false) {src/components/tabs/custom-files-tab.tsx (1)
50-93: Consider improving error handling for the listener setup.The scan progress listener is well-implemented. However, the error handling could be improved by notifying the user when the listener fails to set up.
Consider propagating the error to the UI:
} catch (error) { console.error('Failed to set up scan progress listener:', error); + setError('Failed to set up scan progress listener'); return () => {}; }src-tauri/src/minecraft/mod.rs (3)
844-881: Remove unnecessary async from synchronous function.The implementation correctly checks for both JSON and legacy .lang translation files. However, the function doesn't perform any async operations.
-pub async fn check_mod_translation_exists( +pub fn check_mod_translation_exists( mod_path: &str, mod_id: &str, target_language: &str, ) -> Result<bool, String> {
884-909: Remove unnecessary async and improve error messages.The implementation correctly checks for quest translation files, but has the same async issue.
-pub async fn check_quest_translation_exists( +pub fn check_quest_translation_exists( quest_path: &str, target_language: &str, ) -> Result<bool, String> { let path = PathBuf::from(quest_path); - let parent = path.parent().ok_or("Failed to get parent directory")?; + let parent = path.parent().ok_or_else(|| format!("Failed to get parent directory for: {}", quest_path))?; let file_stem = path .file_stem() - .ok_or("Failed to get file stem")? + .ok_or_else(|| format!("Failed to get file stem for: {}", quest_path))? .to_string_lossy();
912-944: Remove unnecessary async from synchronous function.The guidebook translation check is correctly implemented following Patchouli's file structure.
-pub async fn check_guidebook_translation_exists( +pub fn check_guidebook_translation_exists( guidebook_path: &str, mod_id: &str, book_id: &str, target_language: &str, ) -> Result<bool, String> {src/components/tabs/common/translation-tab.tsx (2)
217-228: Consider using a more unique separator for structured errors.The error handling improvements are good, but the
:::separator might appear in actual error messages or paths.Consider using a more unique separator or a structured error format:
- if (errorMessage.startsWith('errors.') && errorMessage.includes(':::')) { - const [translationKey, path] = errorMessage.split(':::'); + if (errorMessage.startsWith('errors.') && errorMessage.includes('|PATH|')) { + const [translationKey, path] = errorMessage.split('|PATH|');
514-515: Consider performance impact of staggered animations with large datasets.The horizontal scrolling and animations are nice UX improvements. However, the staggered animation might cause performance issues with hundreds of items.
Consider limiting animations to the first N visible items:
- style={{ animationDelay: `${index * 50}ms` }}> + style={{ animationDelay: `${Math.min(index, 10) * 50}ms` }}>Also applies to: 640-640, 659-659
src-tauri/src/filesystem.rs (1)
89-158: Consider adding error handling for progress event emission.The progress tracking implementation looks good. However, if an error occurs during directory traversal, the completion event might not be emitted, leaving the UI in a "scanning" state.
Consider wrapping the directory traversal in a result handler to ensure the completion event is always emitted:
+ let scan_result = (|| -> std::result::Result<Vec<String>, String> { // Walk through the directory and find all JAR files let mut processed_count = 0; let mut last_emit = Instant::now(); const EMIT_INTERVAL: Duration = Duration::from_millis(200); for entry in WalkDir::new(target_dir) .max_depth(1) .into_iter() .filter_map(|e| e.ok()) { // ... existing code ... } + Ok(mod_files) + })(); // Emit completion event let _ = app_handle.emit( "scan_progress", ScanProgressPayload { current_file: "".to_string(), processed_count, total_count: Some(total_files), scan_type: "mods".to_string(), completed: true, }, ); - debug!("Found {} mod files", mod_files.len()); - Ok(mod_files) + match scan_result { + Ok(files) => { + debug!("Found {} mod files", files.len()); + Ok(files) + } + Err(e) => Err(e) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
public/locales/en/common.json(3 hunks)public/locales/ja/common.json(3 hunks)src-tauri/Cargo.toml(2 hunks)src-tauri/src/backup.rs(5 hunks)src-tauri/src/config.rs(4 hunks)src-tauri/src/filesystem.rs(15 hunks)src-tauri/src/lib.rs(3 hunks)src-tauri/src/logging.rs(1 hunks)src-tauri/src/minecraft/mod.rs(3 hunks)src-tauri/src/tests/lang_file_test.rs(1 hunks)src/__tests__/e2e/skip-existing-translations-e2e.test.ts(1 hunks)src/app/page.tsx(3 hunks)src/components/layout/header.tsx(1 hunks)src/components/layout/main-layout.tsx(1 hunks)src/components/settings/settings-dialog.tsx(1 hunks)src/components/settings/translation-settings.tsx(2 hunks)src/components/tabs/common/translation-tab.tsx(11 hunks)src/components/tabs/custom-files-tab.tsx(6 hunks)src/components/tabs/guidebooks-tab.tsx(6 hunks)src/components/tabs/mods-tab.tsx(6 hunks)src/components/tabs/quests-tab.tsx(8 hunks)src/components/ui/button.tsx(1 hunks)src/components/ui/card.tsx(1 hunks)src/components/ui/checkbox.tsx(1 hunks)src/components/ui/completion-dialog.tsx(5 hunks)src/components/ui/dialog.tsx(2 hunks)src/components/ui/input.tsx(1 hunks)src/components/ui/log-dialog.tsx(2 hunks)src/components/ui/progress.tsx(1 hunks)src/components/ui/scroll-area.tsx(2 hunks)src/components/ui/select.tsx(2 hunks)src/components/ui/spinner.tsx(1 hunks)src/components/ui/switch.tsx(1 hunks)src/components/ui/tabs.tsx(2 hunks)src/components/ui/translation-history-dialog.tsx(12 hunks)src/lib/constants/defaults.ts(2 hunks)src/lib/services/config-service.ts(1 hunks)src/lib/store/index.ts(2 hunks)src/lib/types/config.ts(5 hunks)src/lib/utils/path-utils.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
src-tauri/src/config.rs (1)
src-tauri/src/filesystem.rs (1)
serialize_json_sorted(820-824)
src/components/ui/tabs.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
src/components/ui/switch.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
src-tauri/src/backup.rs (1)
src-tauri/src/filesystem.rs (1)
serialize_json_sorted(820-824)
src/components/ui/spinner.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
src-tauri/src/lib.rs (2)
src-tauri/src/logging.rs (1)
read_session_log(557-578)src-tauri/src/minecraft/mod.rs (3)
check_guidebook_translation_exists(913-944)check_mod_translation_exists(846-881)check_quest_translation_exists(885-909)
src/components/ui/scroll-area.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
src/components/tabs/mods-tab.tsx (4)
src/lib/store/index.ts (1)
useAppStore(116-436)src/lib/services/file-service.ts (2)
FileService(188-382)invoke(202-209)src/lib/types/minecraft.ts (2)
TranslationTarget(103-120)ModInfo(13-28)src/lib/utils/path-utils.ts (1)
getRelativePath(42-72)
src/lib/types/config.ts (1)
src/lib/constants/defaults.ts (4)
TRANSLATION_DEFAULTS(36-48)UI_DEFAULTS(117-131)UPDATE_DEFAULTS(136-141)PATH_DEFAULTS(106-112)
🪛 Biome (1.9.4)
src/lib/services/config-service.ts
[error] 154-154: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 154-154: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
⏰ 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 - x86_64-pc-windows-msvc
- GitHub Check: Build - aarch64-apple-darwin
- GitHub Check: Build - x86_64-apple-darwin
- GitHub Check: Build - x86_64-unknown-linux-gnu
- GitHub Check: Validate PR
- GitHub Check: Security Scan
🔇 Additional comments (93)
src-tauri/Cargo.toml (2)
24-24: LGTM: Test feature enabled for Tauri dependency.Adding the "test" feature to the Tauri dependency enables testing capabilities for the backend, which aligns with the PR's testing improvements.
38-40: LGTM: Dev dependencies added for testing support.The
tempfileandtokiodev dependencies provide necessary testing infrastructure for file operations and async testing, which supports the refactored test code mentioned in the AI summary.src/components/layout/header.tsx (2)
79-79: LGTM: Enhanced responsive design for large screens.The addition of 2xl breakpoint classes improves the header layout for very large displays with appropriate max-width, padding, and height adjustments.
80-81: LGTM: Consistent responsive spacing and typography.The gap and font size adjustments maintain visual hierarchy and improve readability on larger screens, consistent with the broader UI responsiveness improvements.
src/components/ui/button.tsx (3)
8-8: LGTM: Enhanced button base styling with responsive design.The comprehensive styling improvements include responsive typography, smooth transitions, active state animations, and better accessibility features. The implementation is well-structured and follows modern design patterns.
13-22: LGTM: Improved visual feedback with shadow effects.The hover and active shadow effects provide excellent visual feedback across all button variants, enhancing the user experience while maintaining consistency.
25-28: LGTM: Consistent responsive sizing across variants.The 2xl breakpoint sizing adjustments ensure buttons scale appropriately on larger screens while maintaining proper proportions and spacing.
src/components/ui/scroll-area.tsx (4)
8-10: LGTM: Well-defined props interface with orientation control.The
ScrollAreaPropsinterface properly extends the base component props and adds the orientation option with appropriate type constraints.
15-17: LGTM: Backward-compatible default orientation.The default "vertical" orientation maintains backward compatibility while supporting the new functionality.
21-21: LGTM: Proper overflow handling for scroll areas.Adding
overflow-hiddento the root container is appropriate for scroll area functionality and prevents content from overflowing unexpectedly.
30-35: LGTM: Conditional scrollbar rendering implementation.The conditional rendering logic correctly shows scrollbars based on the orientation prop, supporting all three modes (vertical, horizontal, both) as intended.
src/lib/utils/path-utils.ts (6)
11-17: LGTM: Robust cross-platform filename extraction.The function correctly handles both Windows and Unix path separators and provides appropriate fallback behavior for edge cases.
24-34: LGTM: Directory path extraction with normalization.The implementation properly extracts directory paths and normalizes them to forward slashes for cross-platform consistency.
79-87: LGTM: Effective path joining with cleanup.The function correctly filters empty segments and normalizes multiple slashes, providing clean path joining functionality.
94-96: LGTM: Simple and effective path normalization.The normalization function is straightforward and serves its purpose for cross-platform compatibility.
104-118: LGTM: Comprehensive absolute path detection.The function correctly identifies Windows drive letters, UNC paths, and Unix absolute paths, covering all major platforms.
125-135: LGTM: Parent directory extraction with proper normalization.The function correctly handles parent directory calculation while maintaining cross-platform compatibility through normalization.
src/components/ui/card.tsx (1)
13-13: Good UX enhancement with smooth hover transitions.The addition of transition effects and hover shadow enhancement improves the interactive feel of card components. The 200ms duration provides a good balance between responsiveness and smoothness.
src/components/layout/main-layout.tsx (1)
54-54: Excellent responsive design enhancement for larger screens.The progressive padding and max-width adjustments for xl and 2xl breakpoints will provide better utilization of space on larger displays while maintaining good readability and layout proportions.
src/components/ui/progress.tsx (1)
24-24: Perfect animation timing for progress indicators.The 300ms duration with ease-out timing provides smooth and natural progress bar animations, which is especially important for the new file count-based progress bars being implemented in this PR.
src/lib/services/config-service.ts (1)
154-154: Correct fix for static method invocation.Good catch! Using the class name instead of
thisfor static method calls is the proper approach and eliminates the confusingthisreference in a static context.src/components/ui/select.tsx (2)
40-40: Enhanced select trigger interactions with smooth transitions.The addition of transition effects for color, box-shadow, and border-color properties, along with the hover and open state styling, significantly improves the visual feedback and user experience of the select component.
110-110: Smooth color transitions for select items.The color transition with 150ms duration and ease-in-out timing provides polished visual feedback when users interact with select items, contributing to a cohesive and responsive UI experience.
src/components/ui/completion-dialog.tsx (1)
91-91: LGTM: Consistent responsive styling improvementsThe responsive styling updates for the 2xl breakpoint are well-implemented and consistent throughout the component. The changes appropriately scale dialog dimensions, text sizes, spacing, and interactive elements for very large screens, enhancing the user experience across different display sizes.
Also applies to: 121-121, 132-132, 142-142, 145-145, 166-166
src-tauri/src/config.rs (1)
1-1: LGTM: Consistent JSON serialization with sorted keysThe replacement of
serde_json::to_string_prettywithserialize_json_sortedis a good improvement that ensures consistent JSON output formatting across the application. The changes maintain the same error handling patterns while providing sorted key output, which improves readability and consistency of configuration files.Also applies to: 166-166, 206-206, 238-238
src/components/settings/settings-dialog.tsx (1)
88-88: LGTM: Responsive styling improvement and path settings removalThe dialog dimension updates for the 2xl breakpoint are consistent with the responsive design improvements throughout the application. The removal of PathSettings functionality (as mentioned in the AI summary) aligns with the PR objective of centralizing path management through a shared profile directory system.
src/components/ui/tabs.tsx (1)
45-45: LGTM: Enhanced tab interactions with smooth animationsThe styling improvements add excellent interactive feedback to the tabs component:
- The
TabsTriggernow includes comprehensive transitions, hover effects, and active state scaling that provide better user experience- The
TabsContentanimation with fade-in and slide-in effects creates smooth transitions between tabsThese enhancements align well with the overall UI improvements in the PR and follow good UX practices.
Also applies to: 60-60
src/lib/constants/defaults.ts (2)
10-10: LGTM: Corrected OpenAI model identifierThe change from "o4-mini-2025-04-16" to "gpt-4o-mini" corrects the model identifier to use the proper OpenAI naming convention. This ensures compatibility with the OpenAI API.
103-112: LGTM: Added path defaults for consistent configurationThe new
PATH_DEFAULTSconstant provides a centralized location for default directory path values, which aligns with the PR's objective of improving path management. This supports the move to a shared profile directory system and ensures consistent initialization of path configurations.src/components/ui/input.tsx (1)
11-11: LGTM! Consistent responsive design improvements.The 2xl breakpoint enhancements follow a consistent pattern with proper scaling of dimensions, padding, and text sizes. The addition of
border-colorto the transition properties will provide smoother visual feedback during focus states.src/app/page.tsx (4)
21-21: LGTM! Proper integration of scanning state.The addition of
isScanningfrom the app store is correctly implemented alongside the existingisTranslatingstate.
46-54: LGTM! Enhanced loading UI with proper spinner.The loading state now provides better visual feedback with a proper spinner animation and improved styling. The fallback text handling for unmounted/unready states is appropriate.
67-72: LGTM! Consistent scanning state handling.The scanning state check follows the same pattern as the translation state check, providing consistent user experience and preventing tab switching during operations.
80-90: LGTM! Proper tab trigger disabling logic.The tab triggers are correctly disabled during both translation and scanning operations, but only for non-active tabs, allowing users to stay on the current tab.
src/components/ui/log-dialog.tsx (2)
330-330: LGTM! Consistent responsive dialog sizing.The 2xl breakpoint enhancements expand the dialog appropriately for very large screens, improving usability and readability.
345-345: LGTM! Improved readability on large screens.The increased padding and text size at the 2xl breakpoint will improve log readability on very large displays.
src/components/ui/spinner.tsx (1)
1-25: Verify Tailwindborder-3utility availabilityThe new spinner at
src/components/ui/spinner.tsxusesborder-3, which isn’t included in Tailwind’s default borderWidth scale. Please ensure yourtailwind.config.*defines a3px (or rem) border width, or adjust to a supported value (e.g.border-2/border-4).• File needing attention:
–src/components/ui/spinner.tsx(sizeClasses.lg usesborder-3)src/components/ui/checkbox.tsx (2)
17-17: Enhanced interactive states improve user experience.The transition from
transition-shadowtotransition-all duration-200 ease-in-outwith added hover and active states creates smoother, more responsive user interactions. The comprehensive className properly handles various states including focus, disabled, and validation states.
24-24: Smooth animation transitions for state changes.The addition of
transition-all duration-200 ease-in-outwith zoom-in/zoom-out animations provides excellent visual feedback for checked/unchecked state transitions. The animation timing is consistent with the root element.src/components/ui/switch.tsx (2)
16-16: Consistent interactive behavior across UI components.The enhanced className with transition timing, hover states, and active scaling creates a polished user experience. The conditional hover states (
hover:data-[state=checked]:bg-primary/90andhover:data-[state=unchecked]:bg-input/70) provide appropriate visual feedback for different switch states.
24-24: Smooth thumb transitions complement the enhanced switch behavior.The addition of
duration-200 ease-in-outto the thumb's transform transition ensures consistent animation timing with the root switch element, creating a cohesive user experience.src-tauri/src/lib.rs (3)
26-26: New session log reading capability added.The import of
read_session_logenables the frontend to access session-specific log files, supporting the new log viewing feature mentioned in the PR objectives.
29-31: Translation existence checking commands support skip functionality.The addition of
check_mod_translation_exists,check_quest_translation_exists, andcheck_guidebook_translation_existscommands enables the frontend to implement the "skip existing translations" feature mentioned in the PR objectives. These commands allow checking for existing language files before processing.
83-85: Properly exposed new commands in Tauri invoke handler.All new commands are correctly added to the
tauri::generate_handler!macro, making them accessible from the frontend. The commands are logically grouped and follow the existing pattern.Also applies to: 121-121
src-tauri/src/logging.rs (1)
555-578: Well-implemented session log reading with proper error handling.The
read_session_logfunction correctly constructs the log file path using the established directory structure and provides appropriate error handling for both missing files and read failures. The error messages are descriptive and will help with debugging.The path construction (
{minecraft_dir}/logs/localizer/{session_id}/localizer.log) aligns with the directory structure used by other functions in this file, ensuring consistency.src/components/settings/translation-settings.tsx (2)
62-62: Minor UI enhancement for visual consistency.The addition of
transition-colors duration-200 hover:bg-muted/70to the token-based chunking container provides subtle visual feedback, consistent with the broader UI improvements across the codebase.
136-156: Well-implemented skip existing translations setting.The new setting follows the established pattern used by other settings in the component:
- Clear label and descriptive hint text using i18n
- Proper Switch component with controlled state
- Correct state management using the config object with spreading
- Default value of
truealigns with the PR objective of skipping already translated filesThe implementation maintains consistency with the existing codebase and provides the expected functionality.
src-tauri/src/backup.rs (3)
1-1: LGTM: Clean import for JSON serialization consistency.The import of
serialize_json_sortedaligns with the project-wide standardization of JSON serialization with sorted keys.
143-146: LGTM: Consistent JSON serialization with sorted keys.The replacement of
serde_json::to_string_prettywithserialize_json_sortedis appropriate and maintains proper error handling while ensuring consistent key ordering in backup metadata.
426-428: LGTM: Consistent JSON serialization for translation summaries.The update maintains proper error handling while ensuring consistent key ordering in translation summary files, completing the JSON serialization standardization in this module.
src/components/ui/dialog.tsx (3)
41-41: LGTM: Enhanced dialog overlay with improved visual effects.The addition of
backdrop-blur-smand consistent animation durations (data-[state=open]:duration-300anddata-[state=closed]:duration-200) provides better visual feedback and follows modern UI patterns where close animations are slightly faster than open animations.
60-60: LGTM: Consistent dialog content animations.The subtle slide animations (
slide-in-from-top-[2%]andslide-out-to-top-[2%]) with matching duration values provide smooth dialog transitions that enhance user experience without being distracting.
66-66: LGTM: Enhanced close button interactivity.The transition to
transition-all duration-200with scale effects (hover:scale-110 active:scale-95) provides excellent tactile feedback that matches the dialog's animation timing and improves user interaction without being overwhelming.src/lib/store/index.ts (3)
98-110: LGTM: Well-structured scanning state interface.The scanning state additions are well-designed:
isScanningflag for simple boolean trackingscanProgressobject with appropriate fields for detailed progress information- Optional fields (
totalCount,scanType) provide flexibilitysetScanProgresswith partial update capability for efficiency- Consistent naming and structure with existing store patterns
394-414: LGTM: Correct implementation of scanning state management.The implementation follows established Zustand patterns:
- Simple boolean setter for
isScanning- Proper initial state with appropriate defaults
setScanProgresscorrectly merges partial updates using spread operatorresetScanProgressproperly resets all fields to initial values- Consistent with existing store patterns and naming conventions
97-97: LGTM: Clean integration with existing state.The scanning state additions are well-integrated without disrupting existing functionality. The separation of concerns between scanning and translation state is appropriate and maintains the store's logical organization.
src-tauri/src/tests/lang_file_test.rs (4)
6-6: LGTM: Appropriate test imports.The import of
serialize_json_sortedandsort_json_objectis correct and necessary for testing the JSON sorting utilities.
8-37: LGTM: Comprehensive test for basic JSON object sorting.The test is well-structured with clear test data and appropriate assertions. It effectively validates that
sort_json_objectcorrectly sorts keys alphabetically in a flat JSON object.
39-79: LGTM: Thorough test for nested JSON object sorting.The test effectively validates recursive key sorting in nested JSON objects. The two-level nesting scenario with appropriate assertions ensures that the
sort_json_objectfunction works correctly for complex data structures.
81-105: LGTM: Excellent end-to-end test for JSON serialization.The test comprehensively validates
serialize_json_sortedby checking both the parsed JSON structure and the string representation. The string position checks are particularly clever for verifying serialization order, ensuring the function works correctly in real-world scenarios.src/lib/types/config.ts (4)
26-40: LGTM: Well-designed PathsConfig interface.The interface is comprehensive and well-documented, providing clear typing for all necessary directory paths. The property names are descriptive and the JSDoc comments enhance maintainability.
116-117: LGTM: Clear addition for translation skipping feature.The
skipExistingTranslationsproperty is well-named, appropriately optional for backward compatibility, and clearly documented. It aligns perfectly with the PR objectives.
54-55: LGTM: Proper integration of PathsConfig interface.The update to use the
PathsConfiginterface improves type safety and provides better structure for path-related configuration.
169-184: LGTM: Proper implementation of new configuration defaults.The default configuration correctly implements the new interfaces:
skipExistingTranslations: trueis a sensible default for avoiding duplicate work- The paths configuration properly uses
PATH_DEFAULTSvalues- The structure maintains consistency with the interface definitions
public/locales/en/common.json (1)
60-63: LGTM! Localization entries are well-structured.The new localization entries for skip existing translations, scan progress, and error messages follow consistent naming conventions and use proper placeholder syntax.
Also applies to: 180-191, 257-260
src/components/tabs/quests-tab.tsx (1)
379-398: Well-implemented language suffix handling.The logic correctly handles:
- Special case for
DefaultQuests.langfiles- Removal of existing language suffixes to prevent duplication
- Proper file extension preservation
Good defensive programming with the regex pattern.
public/locales/ja/common.json (1)
60-63: Japanese translations are consistent and well-formatted.The Japanese translations properly mirror the English locale structure and use appropriate terminology.
Also applies to: 180-191, 257-260
src/components/ui/translation-history-dialog.tsx (9)
3-3: LGTM!The new imports are correctly added and used in the component.
Also applies to: 8-8
89-89: LGTM!The
onViewLogsprop is properly added and the View Logs button is well-implemented with proper icon and localization.Also applies to: 132-143
174-174: LGTM!The
onViewLogsprop is correctly propagated through the component hierarchy.Also applies to: 179-179, 270-270
283-287: LGTM!The state variables for managing the session log dialog are well-structured and properly typed.
295-300: LGTM!Good defensive programming with the directory check and proper error handling. The dependency array is correctly updated.
Also applies to: 323-323
347-374: LGTM!The
handleViewLogscallback is well-implemented with proper state management, error handling, and loading states.
380-398: LGTM!Good performance optimization using
useMemoto prevent unnecessary re-sorting of sessions.
412-412: LGTM!The increased dialog and scroll area sizes improve the user experience on larger screens.
Also applies to: 440-440
496-543: LGTM!The session log dialog is well-structured with proper state handling, loading indicators, error messages, and empty state display.
src/components/tabs/custom-files-tab.tsx (4)
10-12: LGTM!The new imports are properly added for event listening and cross-platform path handling.
41-47: LGTM!The new state variables are properly destructured from the app store for scan progress tracking.
96-161: LGTM!The scan function is properly enhanced with progress tracking and cross-platform path handling. The try-finally block ensures proper cleanup.
175-180: LGTM!Good improvement using path utilities for cross-platform compatibility instead of manual string manipulation.
Also applies to: 294-295
src-tauri/src/minecraft/mod.rs (2)
1-1: LGTM!The import is correctly added for consistent JSON serialization across the codebase.
347-350: LGTM!Good consistency improvement using
serialize_json_sortedfor deterministic JSON output.src/components/tabs/common/translation-tab.tsx (5)
82-89: LGTM!The
scanProgressprop is well-typed and properly structured for displaying scan progress information.
405-419: LGTM!Great UX improvements with loading spinners and pulse animations for active operations.
424-424: LGTM!Consistent and appropriate size adjustments for better user experience on larger screens.
Also applies to: 443-443, 479-479, 492-492, 513-513
558-592: LGTM!Excellent scan progress visualization with comprehensive information display and smooth animations.
467-467: LGTM!Nice UI polish with animations and sticky headers for improved user experience.
Also applies to: 473-473, 519-519
src-tauri/src/filesystem.rs (6)
50-59: LGTM! Well-structured progress tracking payload.The
ScanProgressPayloadstruct is properly designed with appropriate serialization attributes for JavaScript interoperability.
412-547: LGTM! Comprehensive progress tracking implementation.The function properly tracks progress for both Better Quests file locations and correctly calculates the total file count including the potential DefaultQuests.lang file.
571-639: LGTM! Clean implementation of generic file scanning with progress tracking.The function properly implements progress tracking and handles extension filtering correctly, including trimming leading dots from the extension parameter.
799-824: LGTM! Well-implemented JSON sorting utilities.These functions ensure consistent JSON output by recursively sorting object keys, which is important for version control and reproducible builds.
881-885: LGTM! Consistent JSON serialization for language files.Using
serialize_json_sortedensures language files have consistent key ordering, which is beneficial for version control and file comparison.
71-71: Ensure frontend error parsing supports new prefixed formatI wasn’t able to find any existing handlers that parse error strings like
errors.profileDirectoryNotFound:::{dir}in the TypeScript/JavaScript code. Please verify or implement support in the frontend to:
- Detect and split the error key (e.g.
profileDirectoryNotFound) from its parameters- Map the key to a user-friendly message (e.g. via your localization system)
- Display any dynamic parameters (the
{dir}value) in the UI
| import { describe, test, expect } from 'bun:test'; | ||
|
|
||
| describe('Skip Existing Translations E2E - Basic Tests', () => { | ||
| test('should have working test framework', () => { | ||
| expect(true).toBe(true); | ||
| }); | ||
|
|
||
| test('should handle configuration setting', () => { | ||
| const config = { | ||
| translation: { | ||
| skipExistingTranslations: true | ||
| } | ||
| }; | ||
|
|
||
| expect(config.translation.skipExistingTranslations).toBe(true); | ||
| }); | ||
|
|
||
| test('should default to true when skipExistingTranslations is undefined', () => { | ||
| const config = { | ||
| translation: { | ||
| // skipExistingTranslations is undefined | ||
| } | ||
| }; | ||
|
|
||
| const skipExisting = config.translation.skipExistingTranslations ?? true; | ||
| expect(skipExisting).toBe(true); | ||
| }); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename to unit tests and consider adding proper E2E coverage.
These tests are currently basic unit tests rather than end-to-end tests. They validate configuration handling but don't test the actual skip existing translations functionality in a real application context.
Consider the following improvements:
- Rename the file to
skip-existing-translations.test.tssince these are unit tests - Add proper E2E tests that would test:
- UI toggle behavior in translation settings
- Backend API calls to check for existing translations
- Actual skipping behavior during scan operations
- Integration with the scanning progress system
-describe('Skip Existing Translations E2E - Basic Tests', () => {
+describe('Skip Existing Translations - Configuration Tests', () => {For actual E2E testing, you might want to add tests that:
- Mock the backend translation check commands
- Test the UI toggle in translation settings
- Verify that files are actually skipped during scanning
- Test the interaction with the new scanning progress system
📝 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.
| import { describe, test, expect } from 'bun:test'; | |
| describe('Skip Existing Translations E2E - Basic Tests', () => { | |
| test('should have working test framework', () => { | |
| expect(true).toBe(true); | |
| }); | |
| test('should handle configuration setting', () => { | |
| const config = { | |
| translation: { | |
| skipExistingTranslations: true | |
| } | |
| }; | |
| expect(config.translation.skipExistingTranslations).toBe(true); | |
| }); | |
| test('should default to true when skipExistingTranslations is undefined', () => { | |
| const config = { | |
| translation: { | |
| // skipExistingTranslations is undefined | |
| } | |
| }; | |
| const skipExisting = config.translation.skipExistingTranslations ?? true; | |
| expect(skipExisting).toBe(true); | |
| }); | |
| }); | |
| import { describe, test, expect } from 'bun:test'; | |
| describe('Skip Existing Translations - Configuration Tests', () => { | |
| test('should have working test framework', () => { | |
| expect(true).toBe(true); | |
| }); | |
| test('should handle configuration setting', () => { | |
| const config = { | |
| translation: { | |
| skipExistingTranslations: true | |
| } | |
| }; | |
| expect(config.translation.skipExistingTranslations).toBe(true); | |
| }); | |
| test('should default to true when skipExistingTranslations is undefined', () => { | |
| const config = { | |
| translation: { | |
| // skipExistingTranslations is undefined | |
| } | |
| }; | |
| const skipExisting = config.translation.skipExistingTranslations ?? true; | |
| expect(skipExisting).toBe(true); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In src/__tests__/e2e/skip-existing-translations-e2e.test.ts lines 1 to 28, the
current tests are basic unit tests validating configuration but not actual
end-to-end functionality. Rename the file to skip-existing-translations.test.ts
to reflect unit testing. Then create a new E2E test file that covers UI toggle
behavior for skipExistingTranslations, mocks backend API calls to check for
existing translations, verifies that files are skipped during scanning, and
tests integration with the scanning progress system to ensure full end-to-end
coverage.
| /// Validate session ID format: YYYY-MM-DD_HH-MM-SS | ||
| fn validate_session_id_format(session_id: &str) -> bool { | ||
| if session_id.len() != 19 { | ||
| return false; | ||
| } | ||
|
|
||
| let chars: Vec<char> = session_id.chars().collect(); | ||
|
|
||
| // Check pattern: YYYY-MM-DD_HH-MM-SS | ||
| chars[4] == '-' && | ||
| chars[7] == '-' && | ||
| chars[10] == '_' && | ||
| chars[13] == '-' && | ||
| chars[16] == '-' && | ||
| chars[0..4].iter().all(|c| c.is_ascii_digit()) && | ||
| chars[5..7].iter().all(|c| c.is_ascii_digit()) && | ||
| chars[8..10].iter().all(|c| c.is_ascii_digit()) && | ||
| chars[11..13].iter().all(|c| c.is_ascii_digit()) && | ||
| chars[14..16].iter().all(|c| c.is_ascii_digit()) && | ||
| chars[17..19].iter().all(|c| c.is_ascii_digit()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve validation function for better maintainability and efficiency.
The function correctly validates the session ID format, but has several areas for improvement:
- Efficiency: Converting to
Vec<char>for every validation is wasteful for a simple string format check. - Readability: The manual position checking is verbose and error-prone.
- Semantic validation: The function doesn't validate actual date/time ranges (e.g., month 13 would pass).
Consider this more efficient and readable implementation:
-/// Validate session ID format: YYYY-MM-DD_HH-MM-SS
-fn validate_session_id_format(session_id: &str) -> bool {
- if session_id.len() != 19 {
- return false;
- }
-
- let chars: Vec<char> = session_id.chars().collect();
-
- // Check pattern: YYYY-MM-DD_HH-MM-SS
- chars[4] == '-' &&
- chars[7] == '-' &&
- chars[10] == '_' &&
- chars[13] == '-' &&
- chars[16] == '-' &&
- chars[0..4].iter().all(|c| c.is_ascii_digit()) &&
- chars[5..7].iter().all(|c| c.is_ascii_digit()) &&
- chars[8..10].iter().all(|c| c.is_ascii_digit()) &&
- chars[11..13].iter().all(|c| c.is_ascii_digit()) &&
- chars[14..16].iter().all(|c| c.is_ascii_digit()) &&
- chars[17..19].iter().all(|c| c.is_ascii_digit())
-}
+/// Validate session ID format: YYYY-MM-DD_HH-MM-SS
+fn validate_session_id_format(session_id: &str) -> bool {
+ if session_id.len() != 19 {
+ return false;
+ }
+
+ let bytes = session_id.as_bytes();
+
+ // Check separators at expected positions
+ if bytes[4] != b'-' || bytes[7] != b'-' || bytes[10] != b'_' ||
+ bytes[13] != b'-' || bytes[16] != b'-' {
+ return false;
+ }
+
+ // Check that all other positions are ASCII digits
+ let digit_ranges = [(0, 4), (5, 7), (8, 10), (11, 13), (14, 16), (17, 19)];
+ digit_ranges.iter().all(|&(start, end)| {
+ bytes[start..end].iter().all(|&b| b.is_ascii_digit())
+ })
+}Alternative approach using regex (if regex dependency is acceptable):
+use regex::Regex;
+
+/// Validate session ID format: YYYY-MM-DD_HH-MM-SS
+fn validate_session_id_format(session_id: &str) -> bool {
+ static REGEX: std::sync::OnceLock<Regex> = std::sync::OnceLock::new();
+ let regex = REGEX.get_or_init(|| {
+ Regex::new(r"^\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}$").unwrap()
+ });
+ regex.is_match(session_id)
+}📝 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.
| /// Validate session ID format: YYYY-MM-DD_HH-MM-SS | |
| fn validate_session_id_format(session_id: &str) -> bool { | |
| if session_id.len() != 19 { | |
| return false; | |
| } | |
| let chars: Vec<char> = session_id.chars().collect(); | |
| // Check pattern: YYYY-MM-DD_HH-MM-SS | |
| chars[4] == '-' && | |
| chars[7] == '-' && | |
| chars[10] == '_' && | |
| chars[13] == '-' && | |
| chars[16] == '-' && | |
| chars[0..4].iter().all(|c| c.is_ascii_digit()) && | |
| chars[5..7].iter().all(|c| c.is_ascii_digit()) && | |
| chars[8..10].iter().all(|c| c.is_ascii_digit()) && | |
| chars[11..13].iter().all(|c| c.is_ascii_digit()) && | |
| chars[14..16].iter().all(|c| c.is_ascii_digit()) && | |
| chars[17..19].iter().all(|c| c.is_ascii_digit()) | |
| } | |
| /// Validate session ID format: YYYY-MM-DD_HH-MM-SS | |
| fn validate_session_id_format(session_id: &str) -> bool { | |
| if session_id.len() != 19 { | |
| return false; | |
| } | |
| let bytes = session_id.as_bytes(); | |
| // Check separators at expected positions | |
| if bytes[4] != b'-' || bytes[7] != b'-' || bytes[10] != b'_' | |
| || bytes[13] != b'-' || bytes[16] != b'-' { | |
| return false; | |
| } | |
| // Check that all other positions are ASCII digits | |
| let digit_ranges = [(0, 4), (5, 7), (8, 10), (11, 13), (14, 16), (17, 19)]; | |
| digit_ranges.iter().all(|&(start, end)| { | |
| bytes[start..end].iter().all(|&b| b.is_ascii_digit()) | |
| }) | |
| } |
🤖 Prompt for AI Agents
In src-tauri/src/backup.rs between lines 14 and 34, the
validate_session_id_format function inefficiently converts the input string to a
Vec<char> and manually checks each character position, which is verbose and
error-prone. To fix this, replace the manual checks with a regex pattern that
matches the exact format YYYY-MM-DD_HH-MM-SS, improving readability and
maintainability. Additionally, enhance the function to parse the date and time
components and validate their semantic correctness (e.g., month between 1 and
12, day valid for the month, hour between 0 and 23, etc.) to ensure the session
ID represents a valid datetime.
Apply automatic formatting to ensure consistency with Rust style guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/pr-validation.yml (1)
28-178: Fix YAML formatting issues.The static analysis tool detected multiple trailing spaces and a missing newline at the end of the file. These formatting issues should be addressed for consistency.
Apply this diff to fix the formatting issues:
- + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - fi + fi +Remove trailing spaces from all lines and add a newline at the end of the file.
🧹 Nitpick comments (3)
src/lib/types/config.ts (1)
151-151: Consider adding theskipExistingTranslationsdefault to theTRANSLATION_DEFAULTSconstant.The default value
truemakes sense for performance optimization, but it should be defined in theTRANSLATION_DEFAULTSconstant for consistency with other translation configuration defaults.Apply this change to maintain consistency:
// In src/lib/constants/defaults.ts export const TRANSLATION_DEFAULTS = { modChunkSize: 50, questChunkSize: 1, guidebookChunkSize: 1, resourcePackName: "MinecraftModsLocalizer", useTokenBasedChunking: false, maxTokensPerChunk: 3000, fallbackToEntryBased: true, maxEntriesPerChunk: 100, contentSplitThreshold: 1000, splitPartMaxLength: 800, maxFailedChunksRatio: 0.1, + skipExistingTranslations: true, } as const;Then update the default config to use the constant:
- skipExistingTranslations: true + skipExistingTranslations: TRANSLATION_DEFAULTS.skipExistingTranslationssrc/components/ui/translation-history-dialog.tsx (1)
469-473: Simplify the directory path display logic.The directory path display logic is repetitive and could be simplified for better readability.
Extract the logic into a helper function:
const getDisplayPath = (path: string): string => { return path.startsWith('NATIVE_DIALOG:') ? path.substring('NATIVE_DIALOG:'.length) : path; }; // Then use it in the JSX: <div className="text-sm text-muted-foreground"> {t('misc.selectedDirectory')} {getDisplayPath(historyDirectory || profileDirectory)} </div>src/components/tabs/common/translation-tab.tsx (1)
342-344: Apply the static analysis suggestion for optional chainingThe static analysis tool correctly identifies that this can be simplified using optional chaining for better readability and safety.
Apply this diff to use optional chaining:
- const actualPath = profileDirectory && profileDirectory.startsWith("NATIVE_DIALOG:") - ? profileDirectory.substring("NATIVE_DIALOG:".length) - : profileDirectory || ""; + const actualPath = profileDirectory?.startsWith("NATIVE_DIALOG:") + ? profileDirectory.substring("NATIVE_DIALOG:".length) + : profileDirectory || "";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.github/workflows/pr-validation.yml(3 hunks)src-tauri/src/backup.rs(5 hunks)src-tauri/src/filesystem.rs(15 hunks)src-tauri/src/logging.rs(1 hunks)src/__tests__/e2e/fixtures/output/realistic/complexmod.ja_jp.json(1 hunks)src/__tests__/e2e/fixtures/output/simple/getting_started.ja_jp.snbt(1 hunks)src/__tests__/e2e/fixtures/output/simple/samplemod.ja_jp.json(1 hunks)src/components/settings/path-settings.tsx(0 hunks)src/components/tabs/common/translation-tab.tsx(16 hunks)src/components/tabs/settings-tab.tsx(0 hunks)src/components/ui/translation-history-dialog.tsx(11 hunks)src/lib/constants/defaults.ts(2 hunks)src/lib/services/config-service.ts(3 hunks)src/lib/services/translation-runner.ts(1 hunks)src/lib/store/index.ts(4 hunks)src/lib/types/config.ts(2 hunks)
💤 Files with no reviewable changes (2)
- src/components/tabs/settings-tab.tsx
- src/components/settings/path-settings.tsx
✅ Files skipped from review due to trivial changes (3)
- src/tests/e2e/fixtures/output/simple/samplemod.ja_jp.json
- src/tests/e2e/fixtures/output/realistic/complexmod.ja_jp.json
- src/tests/e2e/fixtures/output/simple/getting_started.ja_jp.snbt
🚧 Files skipped from review as they are similar to previous changes (5)
- src/lib/constants/defaults.ts
- src-tauri/src/backup.rs
- src-tauri/src/logging.rs
- src/lib/store/index.ts
- src-tauri/src/filesystem.rs
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/lib/types/config.ts (1)
src/lib/constants/defaults.ts (1)
TRANSLATION_DEFAULTS(36-48)
src/components/ui/translation-history-dialog.tsx (6)
src/components/ui/button.tsx (1)
Button(59-59)src/lib/i18n.ts (1)
useAppTranslation(46-49)src/lib/store/index.ts (1)
useAppStore(120-444)src/lib/services/file-service.ts (1)
FileService(188-382)src/components/ui/dialog.tsx (5)
DialogContent(127-127)DialogHeader(130-130)DialogTitle(133-133)Dialog(125-125)DialogFooter(129-129)src/components/ui/scroll-area.tsx (1)
ScrollArea(68-68)
src/lib/services/config-service.ts (1)
src/lib/types/config.ts (1)
DEFAULT_CONFIG(126-159)
🪛 YAMLlint (1.37.1)
.github/workflows/pr-validation.yml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 51-51: trailing spaces
(trailing-spaces)
[error] 66-66: trailing spaces
(trailing-spaces)
[error] 71-71: trailing spaces
(trailing-spaces)
[error] 84-84: trailing spaces
(trailing-spaces)
[error] 89-89: trailing spaces
(trailing-spaces)
[error] 92-92: trailing spaces
(trailing-spaces)
[error] 95-95: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 113-113: trailing spaces
(trailing-spaces)
[error] 116-116: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 139-139: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 156-156: trailing spaces
(trailing-spaces)
[error] 161-161: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 169-169: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 178-178: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Biome (1.9.4)
src/components/tabs/common/translation-tab.tsx
[error] 342-342: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/lib/services/config-service.ts
[error] 154-154: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 154-154: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
⏰ 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-apple-darwin
- GitHub Check: Build - x86_64-unknown-linux-gnu
- GitHub Check: Build - x86_64-pc-windows-msvc
- GitHub Check: Security Scan
- GitHub Check: Backend Tests
🔇 Additional comments (29)
src/lib/types/config.ts (1)
97-98: Good addition of theskipExistingTranslationsfeature flag.This new optional property provides users with control over whether to skip files that already have existing translations, which can significantly improve performance during translation operations.
.github/workflows/pr-validation.yml (1)
8-60: Excellent workflow restructuring for better separation of concerns.The split into separate jobs (
frontend-tests,backend-tests,integration-tests,security-scan) improves maintainability and allows for parallel execution. The caching strategy is also well-implemented.src/lib/services/translation-runner.ts (1)
160-163: LGTM! Proper migration to shared profile directory system.The change from
config.paths.minecraftDirtouseAppStore.getState().profileDirectoryaligns with the PR's objective of removing path settings and using a shared profile directory system. The fallback to empty string and maintaining theminecraftDirkey for backend compatibility is appropriate.src/lib/services/config-service.ts (2)
154-154: Correct fix for static method invocation.The change from
this.migrateApiKeys(this.config)toConfigService.migrateApiKeys(this.config)is correct sincemigrateApiKeysis a static method. This fixes the static analysis warning about usingthisin a static context.
398-400: Good addition of update configuration handling.Adding the
updateconfiguration withcheckOnStartupprovides proper handling for update-related settings in the config conversion process.src/components/ui/translation-history-dialog.tsx (6)
3-3: Good addition of necessary imports for new functionality.The new imports (
useMemo,FileText,FolderOpen,FileService) are appropriate for the enhanced session log viewing and directory selection features.Also applies to: 8-8, 12-12
284-289: Well-structured state management for session log dialog.The state variables for managing the session log dialog are properly named and initialized. The separation of concerns between loading, error, and content states is good.
344-365: Excellent directory selection implementation with proper error handling.The directory selection handler includes proper validation, error handling, and automatic session reloading. The user experience is well thought out.
383-415: Comprehensive session log viewing implementation.The session log viewing functionality is well-implemented with proper loading states, error handling, and fallback logic for directory selection. The async handling is appropriate.
422-440: Good use of useMemo for performance optimization.The memoization of the sorted sessions prevents unnecessary re-computations and improves performance. The sorting logic is clean and handles all the required fields.
560-607: Well-structured session log dialog with proper UX.The session log dialog provides a good user experience with loading states, error handling, and proper content display. The scrollable pre-formatted content is appropriate for log viewing.
src/components/tabs/common/translation-tab.tsx (18)
21-22: LGTM: Good addition of necessary importsThe addition of
useAppStoreandtoastimports aligns well with the new shared state management and enhanced error handling functionality.
84-92: LGTM: Well-designed scan progress interfaceThe optional
scanProgressprop provides a clean interface for tracking scan progress with all necessary fields for displaying current file, counts, and scan type.
158-160: LGTM: Proper integration with shared stateThe migration from local state to shared
profileDirectorystate from the global store is correctly implemented and will improve consistency across tabs.
173-177: LGTM: Good input validationThe directory path validation with proper error handling ensures invalid selections are caught early with appropriate user feedback.
199-204: LGTM: Enhanced error handling with toast notificationsThe improved error handling combines both component state updates and toast notifications for better user experience, with proper error message extraction.
209-213: LGTM: Proper validation before scanningThe validation ensures a profile directory is selected before attempting to scan, with appropriate error messages and toast notifications.
224-229: LGTM: Smart UI optimization with requestAnimationFrameUsing
requestAnimationFrameto defer UI clearing operations prevents blocking the main thread during scanning operations, improving responsiveness.
234-247: LGTM: Sophisticated error message parsingThe error handling logic properly handles both translation keys with path parameters and regular error messages, providing contextual feedback to users.
420-428: LGTM: Enhanced button states with loading indicatorsThe scan button improvements with loading spinner, pulse animation, and proper disabled states provide excellent user feedback during operations.
432-438: LGTM: Consistent loading indicatorsThe translate button follows the same pattern as the scan button with appropriate loading states and animations.
441-441: LGTM: Responsive design improvementThe responsive width classes ensure the target language selector adapts properly to different screen sizes.
460-460: LGTM: Consistent responsive designThe search input follows the same responsive design pattern as other components.
475-481: LGTM: Proper directory path displayThe directory display correctly handles the NATIVE_DIALOG prefix removal for clean user presentation.
484-487: LGTM: Animated error messagesThe error message styling with fade-in and slide-in animations provides smooth visual feedback.
490-527: LGTM: Enhanced translation progress displayThe translation progress section with dual progress bars (individual and overall) provides comprehensive feedback during translation operations.
530-532: LGTM: Improved table containerThe ScrollArea with both orientations and responsive height constraints provides better table handling across different screen sizes.
575-610: LGTM: Comprehensive scan progress UIThe scanning progress display with file information, counts, percentage, and progress bar provides excellent user feedback during scan operations. The responsive design and animations enhance the user experience.
657-657: LGTM: Staggered animation for table rowsThe table row animations with staggered delays create a smooth, professional appearance when results are displayed.
| const actualPath = minecraftDir.startsWith('NATIVE_DIALOG:') | ||
| ? minecraftDir.substring('NATIVE_DIALOG:'.length) | ||
| : minecraftDir; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract NATIVE_DIALOG prefix handling into a utility function.
The NATIVE_DIALOG: prefix stripping logic is repeated in multiple places. This should be extracted into a reusable utility function to avoid code duplication.
Create a utility function:
// Add to src/lib/utils/path-utils.ts or similar
export const extractActualPath = (path: string): string => {
return path.startsWith('NATIVE_DIALOG:')
? path.substring('NATIVE_DIALOG:'.length)
: path;
};Then replace the repeated logic:
- // Extract the actual path if it has the NATIVE_DIALOG prefix
- const actualPath = minecraftDir.startsWith('NATIVE_DIALOG:')
- ? minecraftDir.substring('NATIVE_DIALOG:'.length)
- : minecraftDir;
+ const actualPath = extractActualPath(minecraftDir);Also applies to: 400-402
🤖 Prompt for AI Agents
In src/components/ui/translation-history-dialog.tsx around lines 305 to 307 and
also lines 400 to 402, the logic to strip the 'NATIVE_DIALOG:' prefix from paths
is duplicated. Extract this logic into a utility function named
extractActualPath in a new or existing file like src/lib/utils/path-utils.ts.
Replace the inline prefix stripping code with calls to this new utility function
to remove duplication and improve maintainability.
- Set main layout container max width to 1600px on 2xl screens - Add xl breakpoint with max-w-6xl for better responsive scaling - Limit translation history dialog to 1600px max width on large screens - Reduce dialog width from 95vw to 90vw for better margins
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/components/ui/translation-history-dialog.tsx (2)
305-307: Extract NATIVE_DIALOG prefix handling into a utility function.The
NATIVE_DIALOG:prefix stripping logic is duplicated in multiple places. This should be extracted into a reusable utility function to avoid code duplication.Create a utility function:
// Add to src/lib/utils/path-utils.ts or similar export const extractActualPath = (path: string): string => { return path.startsWith('NATIVE_DIALOG:') ? path.substring('NATIVE_DIALOG:'.length) : path; };Then replace the repeated logic:
- // Extract the actual path if it has the NATIVE_DIALOG prefix - const actualPath = minecraftDir.startsWith('NATIVE_DIALOG:') - ? minecraftDir.substring('NATIVE_DIALOG:'.length) - : minecraftDir; + const actualPath = extractActualPath(minecraftDir);
400-402: Extract NATIVE_DIALOG prefix handling into a utility function.The
NATIVE_DIALOG:prefix stripping logic is duplicated in multiple places. This should be extracted into a reusable utility function to avoid code duplication.Create a utility function:
// Add to src/lib/utils/path-utils.ts or similar export const extractActualPath = (path: string): string => { return path.startsWith('NATIVE_DIALOG:') ? path.substring('NATIVE_DIALOG:'.length) : path; };Then replace the repeated logic:
- // Extract the actual path if it has the NATIVE_DIALOG prefix - const actualPath = minecraftDir.startsWith('NATIVE_DIALOG:') - ? minecraftDir.substring('NATIVE_DIALOG:'.length) - : minecraftDir; + const actualPath = extractActualPath(minecraftDir);
🧹 Nitpick comments (1)
src/components/ui/translation-history-dialog.tsx (1)
90-90: Enhance type safety by adding proper prop types.The component prop interface should be more explicitly defined for better type safety.
-function SessionDetailsRow({ sessionSummary, onViewLogs }: { sessionSummary: SessionSummary; onViewLogs: (sessionId: string) => void }) { +interface SessionDetailsRowProps { + sessionSummary: SessionSummary; + onViewLogs: (sessionId: string) => void; +} + +function SessionDetailsRow({ sessionSummary, onViewLogs }: SessionDetailsRowProps) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/layout/main-layout.tsx(1 hunks)src/components/ui/translation-history-dialog.tsx(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/layout/main-layout.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/ui/translation-history-dialog.tsx (6)
src/components/ui/button.tsx (1)
Button(59-59)src/lib/i18n.ts (1)
useAppTranslation(46-49)src/lib/store/index.ts (1)
useAppStore(120-444)src/lib/services/file-service.ts (1)
FileService(188-382)src/components/ui/dialog.tsx (5)
DialogContent(127-127)DialogHeader(130-130)DialogTitle(133-133)Dialog(125-125)DialogFooter(129-129)src/components/ui/scroll-area.tsx (1)
ScrollArea(68-68)
⏰ 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 - x86_64-apple-darwin
- GitHub Check: Build - x86_64-pc-windows-msvc
- GitHub Check: Build - aarch64-apple-darwin
- GitHub Check: Build - x86_64-unknown-linux-gnu
- GitHub Check: Security Scan
- GitHub Check: Backend Tests
🔇 Additional comments (16)
src/components/ui/translation-history-dialog.tsx (16)
3-3: Good addition of useMemo import for performance optimization.The addition of
useMemoimport is appropriate for optimizing the session sorting performance.
8-8: Approve new icon imports for enhanced UI.The new
FileTextandFolderOpenicons are appropriately imported and will enhance the user interface for the new features.
12-12: Good integration of FileService for directory selection.The FileService import is correctly added to support the new directory selection functionality.
133-144: Well-implemented View Logs button with proper accessibility.The View Logs button implementation is well-structured with proper icon placement, accessibility features, and clear user interaction patterns.
175-181: Good prop interface expansion for SessionRow.The addition of the
onViewLogsprop to the SessionRow component is implemented correctly and maintains type safety.
271-271: Proper callback propagation in SessionDetailsRow.The
onViewLogscallback is correctly passed to the SessionDetailsRow component.
279-279: Correct usage of app store for profile directory.The integration with the app store to retrieve the profile directory is implemented correctly.
284-289: Well-structured state management for session log dialog.The new state variables for managing the session log dialog are appropriately initialized and follow React best practices.
343-365: Robust directory selection implementation with proper error handling.The directory selection functionality is well-implemented with proper validation, error handling, and user feedback. The automatic session reload after directory selection provides good UX.
383-415: Comprehensive session log viewing functionality.The
handleViewLogsfunction is well-implemented with proper error handling, loading states, and fallback directory logic. The async implementation follows React best practices.
422-440: Excellent performance optimization with useMemo.The session sorting is properly memoized to avoid unnecessary re-computations. The sorting logic is comprehensive and handles all relevant fields correctly.
454-454: Good responsive design improvements.The dialog sizing improvements for larger screens (2xl breakpoint) provide better user experience on different screen sizes.
456-476: Well-structured directory selection UI.The directory selection UI is well-implemented with proper button styling, icon placement, and directory path display with prefix handling.
502-502: Improved scroll area sizing for better responsiveness.The scroll area height adjustments for different screen sizes provide better user experience across devices.
530-532: Consistent directory path handling with prefix extraction.The directory path handling in the SessionRow component correctly extracts the actual path from the NATIVE_DIALOG prefix.
560-607: Comprehensive session log dialog implementation.The session log dialog is well-implemented with proper loading states, error handling, and content display. The UI structure follows consistent patterns with the main dialog.
Update format strings to use inline arguments to comply with Clippy's
uninlined_format_args lint. This fixes the failing Backend Tests job
in CI by addressing all format\! macro calls that were using the older
"{}", var syntax instead of the newer "{var}" syntax.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src-tauri/src/filesystem.rs(15 hunks)src-tauri/src/minecraft/mod.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src-tauri/src/minecraft/mod.rs
⏰ 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 - x86_64-apple-darwin
- GitHub Check: Build - aarch64-apple-darwin
- GitHub Check: Build - x86_64-unknown-linux-gnu
- GitHub Check: Build - x86_64-pc-windows-msvc
- GitHub Check: Security Scan
- GitHub Check: Backend Tests
🔇 Additional comments (9)
src-tauri/src/filesystem.rs (9)
5-6: New imports for progress tracking look good.The additions of
Duration,Instantfor timing control andEmitterfor event emission are appropriate for the new progress tracking functionality.
50-59: Well-structured progress payload.The
ScanProgressPayloadstruct provides all necessary fields for comprehensive progress tracking. The optionaltotal_countfield allows flexibility for scenarios where the total might not be known upfront.
89-101: Good practice: Count files upfront for accurate progress tracking.Pre-counting the total files before scanning allows for accurate progress percentage calculations. The 200ms emit interval strikes a good balance between responsiveness and performance.
118-137: Intelligent progress emission logic.The condition
processed_count % 10 == 0 || last_emit.elapsed() >= EMIT_INTERVAL || entry_path.extension().is_some_and(|ext| ext == "jar")is well-designed:
- Every 10 files for consistent updates
- Time-based updates every 200ms for responsiveness
- Immediate updates when finding JAR files for user feedback
This provides good UX without overwhelming the event system.
147-158: Consistent completion event emission.The completion event with
completed: trueand emptycurrent_fileprovides a clear signal to the frontend that scanning is finished.
273-283: Progress tracking properly integrated into FTB quest scanning.The same pattern is consistently applied across all scanning functions, which is good for maintainability. The progress tracking is correctly scoped within each quest directory loop.
797-822: Excellent JSON serialization utilities.The recursive
sort_json_objectfunction andserialize_json_sortedwrapper provide consistent JSON output formatting. The implementation correctly handles nested objects and arrays.
879-883: Consistent use of sorted JSON serialization.The
write_lang_filefunction correctly uses the newserialize_json_sortedutility, ensuring consistent JSON output across the application.
64-64: Ignore app_handle in frontend invocationsTauri injects the
app_handleautomatically into Rust commands, so frontend calls toinvoke(and yourFileService.invokewrapper) should continue passing only the payload object. No changes are needed in your TS/JS files or tests—in all cases we only see calls like:FileService.invoke('get_mod_files', { dir: '/some/path' });which remain correct. Please disregard the earlier suggestion to add an
app_handleargument in frontend code.Likely an incorrect or invalid review comment.
| // Count total files first for progress tracking | ||
| let total_files = if better_quests_dir.exists() && better_quests_dir.is_dir() { | ||
| WalkDir::new(&better_quests_dir) | ||
| .max_depth(1) | ||
| .into_iter() | ||
| .filter_map(|e| e.ok()) | ||
| .filter(|entry| entry.path().is_file()) | ||
| .count() | ||
| } else { | ||
| 0 | ||
| } + 1; // +1 for the potential DefaultQuests.lang file | ||
|
|
||
| // Progress tracking | ||
| let mut processed_count = 0; | ||
| let mut last_emit = Instant::now(); | ||
| const EMIT_INTERVAL: Duration = Duration::from_millis(200); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify the total file count calculation for Better Quests.
The total file count calculation adds 1 for the potential DefaultQuests.lang file regardless of whether it exists. This could lead to inaccurate progress percentages if the file doesn't exist.
Consider checking if the file exists before adding it to the count:
- } + 1; // +1 for the potential DefaultQuests.lang file
+ } + if path.join("config").join("betterquesting").join("DefaultQuests.lang").exists() { 1 } else { 0 };Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src-tauri/src/filesystem.rs around lines 412 to 428, the total file count for
Better Quests always adds 1 for the potential DefaultQuests.lang file without
verifying its existence, which can cause inaccurate progress tracking. Modify
the code to check if DefaultQuests.lang actually exists before adding 1 to the
total file count. This ensures the total_files variable accurately reflects the
real number of files to process.
| let path = Path::new(dir); | ||
| if !path.exists() || !path.is_dir() { | ||
| return Err(format!("Directory not found: {dir}")); | ||
| return Err(format!("errors.profileDirectoryNotFound:::{dir}")); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify error message format consistency across the codebase.
The error messages now use a prefixed format like "errors.profileDirectoryNotFound:::{dir}". This appears to be for internationalization purposes, but ensure that the frontend properly handles this new format.
Also applies to: 176-176, 182-182, 402-402, 565-565
🏁 Script executed:
#!/bin/bash
# Description: Check how these prefixed error messages are handled in the frontend
# Expected: Find error handling code that processes the "errors." prefix format
# Search for error handling patterns in frontend code
rg -A 3 -B 3 "errors\." --type=tsx --type=ts --type=js
# Also check for the specific error keys in localization files
rg "profileDirectoryNotFound|questsDirectoryNotFound|guidebooksDirectoryNotFound|customFilesDirectoryNotFound" --type=jsonLength of output: 1461
Inconsistent error message format: add front-end parsing or simplify backend output
The Rust code in src-tauri/src/filesystem.rs (lines 71, 176, 182, 402, 565) now returns errors as:
Err(format!("errors.profileDirectoryNotFound:::{dir}"))but your localization files define flat keys ("profileDirectoryNotFound") under common.json (no nested errors object), and there’s no existing front-end logic to:
- strip the
errors.prefix, - split on
:::to extract the translation key vs. the path, - call
t(translationKey, { path }).
As a result, lookups for "errors.profileDirectoryNotFound" will fail, and the raw string (including :::) will be displayed.
To fix this, choose one of:
- Simplify backend output: return just the key and parameters (e.g.
("profileDirectoryNotFound", dir)orformat!("profileDirectoryNotFound:::{dir}")), and let the front-end split/use it. - Implement front-end parsing: before rendering errors, split on
:::; remove theerrors.prefix; then callt(key, { path }).
Affected backend locations:
- src-tauri/src/filesystem.rs: lines 71, 176, 182, 402, 565
Please adjust accordingly so translation lookups succeed.
🤖 Prompt for AI Agents
In src-tauri/src/filesystem.rs at lines 71, 176, 182, 402, and 565, the error
messages include a prefix "errors." and use a ::: separator which does not match
the flat translation keys in the front-end localization files. To fix this,
modify the backend error returns to only include the translation key without the
"errors." prefix, for example return "profileDirectoryNotFound:::{dir}" instead
of "errors.profileDirectoryNotFound:::{dir}". This will align the backend output
with the front-end keys and enable correct translation lookups without requiring
front-end parsing changes.
Summary
Changes
Progress Bar Implementation (TX022)
Profile Directory Management (TX023)
Technical Improvements
Test Results
All PR validation checks passed:
Screenshots
The scan progress now shows:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Tests
Chores