-
Notifications
You must be signed in to change notification settings - Fork 28
Add structured JSON parallel translation feature #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Added constants for magic numbers (MAX_LINE_BREAKS, SHORT_STRING_LENGTH, PRIORITY_RATIO) - Extracted repeated line counting logic into isVeryLongText() method in both files - Replaced all magic numbers with class constants - Translated all Korean comments to English - Improved code maintainability and readability Fixes issues #2, #6, and #7 from PR review Co-authored-by: Sangrak Choi <[email protected]>
…avel-ai-translator into feat/json-parallel-structred
|
@greptile review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10 files reviewed, 11 comments
| */ | ||
| protected function loadReferenceTranslations(string $file, string $targetLocale, array $sourceStringList): array | ||
| { | ||
| // 타겟 언어와 레퍼런스 언어들을 모두 포함 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Korean comment in English codebase - translate to: "// Include both target language and reference languages"
Context Used: Context from dashboard - .cursorrules (source)
| $references[$referenceLocale] = $referenceStrings; | ||
| } | ||
|
|
||
| // AIProvider 인스턴스 생성 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Korean comment in English codebase - translate to: "// Create AIProvider instance"
Context Used: Context from dashboard - .cursorrules (source)
| { | ||
| $prioritized = []; | ||
|
|
||
| // 1. Short strings first (UI elements, buttons, etc.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Korean comment in English codebase - translate to: "// 1. Short strings first (UI elements, buttons, etc.)"
Context Used: Context from dashboard - .cursorrules (source)
| } | ||
| } | ||
|
|
||
| // 2. Add remaining items |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Korean comment in English codebase - translate to: "// 2. Add remaining items"
Context Used: Context from dashboard - .cursorrules (source)
| '--chunk='.$this->option('chunk-size'), | ||
| '--max-context='.$this->option('max-context'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: 옵션 이름 불일치: 시그니처에서는 --chunk-size인데 여기서는 --chunk 사용. 일관성을 위해 --chunk-size={$this->option('chunk-size')}로 수정 권장
| $timeout = 1800; // 30 minutes | ||
| curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 30); // Keep connection timeout at 30 seconds | ||
| curl_setopt($ch, CURLOPT_TIMEOUT, $timeout); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: 타임아웃 값을 request() 메서드와 동일하게 설정했어요. 두 메서드 간 일관성을 위해 클래스 상수나 설정 값으로 추출하는 것을 고려해보세요.
| ]; | ||
|
|
||
| // Set timeout to 30 minutes for streaming | ||
| $timeout = 1800; // 30 minutes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: 일관성을 위해 중복된 주석과 변수 선언을 제거하는 것을 고려하세요. 44번 라인과 289번 라인이 동일한 값입니다
| // 'max_tokens' => 4096, | ||
| // 'use_extended_thinking' => false, // Extended Thinking 기능 사용 여부 (claude-3-7-sonnet-latest 모델만 지원) | ||
| // 'disable_stream' => true, // Disable streaming mode for better error messages | ||
| 'use_extended_thinking' => true, // Extended Thinking 기능 사용 여부 (claude-3-7-sonnet-latest 모델만 지원) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: use_extended_thinking을 사용하려면 claude-3-7-sonnet 모델이 필요하지만, 12번 라인에서 claude-sonnet-4-20250514로 설정되어 있습니다. 이 옵션은 현재 모델에서는 작동하지 않습니다.
| 'use_extended_thinking' => true, // Extended Thinking 기능 사용 여부 (claude-3-7-sonnet-latest 모델만 지원) | |
| // 'use_extended_thinking' => true, // Extended Thinking 기능 사용 여부 (claude-3-7-sonnet-latest 모델만 지원) |
| { | ||
| $files = []; | ||
|
|
||
| if (!is_dir($directory)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: 일관성 없는 부정 연산자 스타일 - 파일 전체에서는 ! is_dir()(라인 50) 형태를 사용하는데 여기서만 !is_dir()(공백 없음)을 사용함
| if (!is_dir($directory)) { | |
| if (! is_dir($directory)) { |
| } | ||
|
|
||
| // Limit maximum items per file | ||
| $maxPerFile = min(20, intval($maxContextItems / max(count($sourceFiles) / 2, 1)) + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: $maxPerFile 계산이 $sourceFiles 배열 전체를 순회하는 루프 내에서 반복 실행됨 - 루프 전에 한 번만 계산하면 성능 개선
Greptile Overview
Updated On: 2025-10-19 09:57:57 UTC
Greptile Summary
이 PR은 대규모 JSON 번역 작업을 지원하기 위한 구조화된 병렬 번역 기능을 추가합니다. 주요 변경사항으로는 중첩 디렉토리 구조를 처리하는
TranslateJsonStructured및TranslateJsonStructuredParallel명령어 추가, JSON 파일용 전용 컨텍스트 제공자(JSONTranslationContextProvider) 구현, AI 프롬프트의 XML 구조 전환(CDATA 섹션으로 특수문자 처리 개선), Anthropic 캐시 토큰 추적 추가, 그리고 모든 AI 클라이언트의 타임아웃을 30분으로 확장했습니다. 이 변경들은 기존 PHP 언어 파일 번역 워크플로우와 일관된 패턴을 따르며, 병렬 처리, 청킹, 참조 번역, 토큰 사용량 추적 등 패키지의 핵심 기능을 유지합니다. 새로운 명령어들은ServiceProvider에 등록되어 아티즌 커맨드로 사용할 수 있습니다.Changed Files
isTranslated메서드를 수정하여 빈 문자열을 미번역으로 처리Confidence score: 3/5
config/ai-translator.php에서use_extended_thinking이 호환되지 않는 모델(claude-sonnet-4-20250514)과 함께 활성화됨, (2)TranslateJsonStructuredParallel.php의 152번 줄에서--chunk옵션명이 시그니처의--chunk-size와 불일치, (3)dot_notationfalse 변경이 기존 프로젝트의 하위 호환성에 영향, (4)AIProvider.php의 Claude 모델 감지 정규표현식(preg_match('/^claude.*(3\-7|4)/', ...))이 모호하여 의도치 않은 매칭 가능성, (5) 빈 문자열을 미번역으로 처리하는JSONLangTransformer변경이 의도적으로 빈 값을 사용하는 기존 번역에 영향 가능.config/ai-translator.php(모델 호환성 확인 필요),src/Console/TranslateJsonStructuredParallel.php(옵션명 수정 필요),src/AI/AIProvider.php(정규표현식 정밀도 개선 필요).PR Description Notes:
Sequence Diagram
sequenceDiagram participant User participant TranslateJsonStructuredParallel participant Process participant TranslateJsonStructured participant AIProvider participant AnthropicClient participant GeminiClient participant OpenAIClient participant JSONLangTransformer participant JSONTranslationContextProvider User->>TranslateJsonStructuredParallel: "Execute ai-translator:translate-json-structured-parallel" TranslateJsonStructuredParallel->>TranslateJsonStructuredParallel: "Initialize source directory and locale" TranslateJsonStructuredParallel->>TranslateJsonStructuredParallel: "Get and validate target locales" TranslateJsonStructuredParallel->>TranslateJsonStructuredParallel: "Filter out source and skip locales" loop For each locale (parallel, max processes) TranslateJsonStructuredParallel->>Process: "buildLanguageCommand(locale)" TranslateJsonStructuredParallel->>Process: "Start new translation process" Process->>TranslateJsonStructured: "Execute ai-translator:translate-json-structured" TranslateJsonStructured->>TranslateJsonStructured: "Get source JSON files recursively" loop For each JSON file TranslateJsonStructured->>JSONLangTransformer: "Load source strings and flatten" TranslateJsonStructured->>JSONLangTransformer: "Load target strings (if exists)" TranslateJsonStructured->>JSONLangTransformer: "Filter untranslated and skip very long texts" TranslateJsonStructured->>TranslateJsonStructured: "loadReferenceTranslations()" TranslateJsonStructured->>JSONTranslationContextProvider: "getGlobalTranslationContext()" JSONTranslationContextProvider->>JSONTranslationContextProvider: "getAllJsonFiles() recursively" JSONTranslationContextProvider->>JSONTranslationContextProvider: "getPrioritizedStrings()" JSONTranslationContextProvider-->>TranslateJsonStructured: "Return context data" loop For each chunk TranslateJsonStructured->>AIProvider: "Create AIProvider with context" TranslateJsonStructured->>AIProvider: "Set callbacks (onTranslated, onThinking, onTokenUsage)" TranslateJsonStructured->>AIProvider: "translate()" alt Provider is Anthropic AIProvider->>AnthropicClient: "messages().createStream()" AnthropicClient->>AnthropicClient: "requestStream() with 30min timeout" AnthropicClient-->>AIProvider: "Stream translation chunks" else Provider is OpenAI AIProvider->>OpenAIClient: "createChatStream()" OpenAIClient->>OpenAIClient: "requestStream() with 30min timeout" OpenAIClient-->>AIProvider: "Stream translation chunks" else Provider is Gemini AIProvider->>GeminiClient: "request() with 30min timeout" GeminiClient-->>AIProvider: "Return translation" end AIProvider->>AIProvider: "Parse response and verify translations" AIProvider-->>TranslateJsonStructured: "Return translated items" TranslateJsonStructured->>JSONLangTransformer: "updateString() for each translation" JSONLangTransformer->>JSONLangTransformer: "saveToFile() with _comment metadata" end end Process-->>TranslateJsonStructuredParallel: "Process completed/failed status" TranslateJsonStructuredParallel->>TranslateJsonStructuredParallel: "Track completed and failed locales" end TranslateJsonStructuredParallel->>User: "Display final summary with statistics"Context used (3)
dashboard- CLAUDE.md (source)dashboard- .cursorrules (source)dashboard- AGENTS.md (source)