Skip to content

Commit 1471505

Browse files
sjsyrekclaude
andcommitted
feat(validation): add upfront language code validation (Issue #3)
Fixed user experience issue where invalid language codes were only caught during API calls, leading to cryptic error messages. Now validates all language codes upfront before making any API requests. Changes: - Added VALID_LANGUAGES Set with all 30 supported DeepL language codes - Added validateLanguageCodes() method to TranslateCommand class - Validates single language codes (--to es) - Validates multiple language codes (--to es,fr,de) - Validates across all translation paths: text, file, directory, document - Clear error messages list all valid language codes - Empty strings after trimming are properly detected and rejected - Added 6 comprehensive tests covering all validation scenarios Benefits: - Users get immediate, actionable feedback about invalid language codes - Error messages include the full list of 30 valid codes for easy reference - Prevents wasted API calls with invalid parameters - Consistent validation across all translation workflows - Better developer experience with early error detection Test Coverage: - All 1442 tests passing (100% pass rate) - New tests: multi-language validation suite (6 tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 51a0129 commit 1471505

3 files changed

Lines changed: 141 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1616
- **Impact**: Ensures future developers understand the importance of property order
1717
- Location: `src/services/translation.ts:386-417`, test added to `tests/unit/translation-service.test.ts`
1818

19+
- **Language Code Validation** - Upfront validation prevents late API errors (Issue #3)
20+
- Validates all language codes before making API calls
21+
- Applies to single language (`--to es`) and multiple languages (`--to es,fr,de`)
22+
- Validates across all translation workflows: text, file, directory, and document translation
23+
- Clear error messages list all 30 valid language codes
24+
- Empty language codes after trimming are detected and rejected
25+
- **Impact**: Users get immediate feedback about invalid language codes instead of cryptic API errors
26+
- **Example**: `deepl translate "Hello" --to invalid` now shows: "Invalid target language code: "invalid". Valid codes: ar, bg, cs, ..."
27+
- Added 6 comprehensive tests covering single/multi-language validation
28+
- Location: `src/cli/commands/translate.ts:81-92` (validation method), multiple call sites
29+
1930
### Fixed
2031
- **Critical: Duplicate text handling in batch translation** - Fixed data loss bug for duplicate inputs
2132
- When input array contained duplicate texts (e.g., `["Hello", "Hello", "World"]`), only the last occurrence received translation

src/cli/commands/translate.ts

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ import { Language } from '../../types/index.js';
1616
import { formatTranslationJson, formatMultiTranslationJson, formatMultiTranslationTable } from '../../utils/formatters.js';
1717
import { Logger } from '../../utils/logger.js';
1818

19+
// Valid language codes supported by DeepL
20+
const VALID_LANGUAGES: ReadonlySet<string> = new Set([
21+
'ar', 'bg', 'cs', 'da', 'de', 'el', 'en', 'es', 'et', 'fi', 'fr',
22+
'hu', 'id', 'it', 'ja', 'ko', 'lt', 'lv', 'nb', 'nl', 'pl', 'pt',
23+
'ro', 'ru', 'sk', 'sl', 'sv', 'tr', 'uk', 'zh'
24+
]);
25+
1926
// Constants for text-based file caching
2027
const TEXT_BASED_EXTENSIONS = ['.txt', '.md', '.html', '.htm', '.srt', '.xlf', '.xliff'];
2128
const SAFE_TEXT_SIZE_LIMIT = 100 * 1024; // 100 KiB (safe threshold, API limit is 128 KiB)
@@ -71,6 +78,19 @@ export class TranslateCommand {
7178
this.config = config;
7279
}
7380

81+
/**
82+
* Validate language codes
83+
* Throws error if any language code is invalid
84+
* Fix for Issue #3: Validate language codes upfront to provide clear error messages
85+
*/
86+
private validateLanguageCodes(langCodes: string[]): void {
87+
for (const lang of langCodes) {
88+
if (!VALID_LANGUAGES.has(lang)) {
89+
throw new Error(`Invalid target language code: "${lang}". Valid codes: ${Array.from(VALID_LANGUAGES).sort().join(', ')}`);
90+
}
91+
}
92+
}
93+
7494
/**
7595
* Resolve glossary ID from name or ID
7696
* If input looks like a UUID (glossary-*), use it directly as ID
@@ -207,7 +227,13 @@ export class TranslateCommand {
207227

208228
// Check if translating to multiple languages (must be done BEFORE text file optimization)
209229
if (options.to.includes(',')) {
210-
const targetLangs = options.to.split(',').map(lang => lang.trim()) as Language[];
230+
const targetLangs = options.to.split(',').map(lang => lang.trim());
231+
232+
// Validate all language codes (Issue #3)
233+
this.validateLanguageCodes(targetLangs);
234+
235+
// Now safe to cast as Language[]
236+
const validTargetLangs = targetLangs as Language[];
211237

212238
const translationOptions: {
213239
sourceLang?: Language;
@@ -227,11 +253,11 @@ export class TranslateCommand {
227253

228254
const results = await this.fileTranslationService.translateFileToMultiple(
229255
filePath,
230-
targetLangs,
256+
validTargetLangs,
231257
translationOptions
232258
);
233259

234-
return `Translated ${filePath} to ${targetLangs.length} languages:\n` +
260+
return `Translated ${filePath} to ${validTargetLangs.length} languages:\n` +
235261
results.map(r => ` [${r.targetLang}] ${r.outputPath}`).join('\n');
236262
}
237263

@@ -269,6 +295,9 @@ export class TranslateCommand {
269295
return this.translateDocument(filePath, options);
270296
}
271297

298+
// Validate single language code for file translation (Issue #3)
299+
this.validateLanguageCodes([options.to]);
300+
272301
// Single language translation using file translation service
273302
const translationOptions: {
274303
targetLang: Language;
@@ -317,6 +346,9 @@ export class TranslateCommand {
317346
return this.translateToMultiple(text, options);
318347
}
319348

349+
// Validate single language code (Issue #3)
350+
this.validateLanguageCodes([options.to]);
351+
320352
// Build translation options
321353
const translationOptions: {
322354
targetLang: Language;
@@ -429,7 +461,13 @@ export class TranslateCommand {
429461
* Translate to multiple target languages
430462
*/
431463
private async translateToMultiple(text: string, options: TranslateOptions): Promise<string> {
432-
const targetLangs = options.to.split(',').map(lang => lang.trim()) as Language[];
464+
const targetLangs = options.to.split(',').map(lang => lang.trim());
465+
466+
// Validate all language codes (Issue #3)
467+
this.validateLanguageCodes(targetLangs);
468+
469+
// Now safe to cast as Language[]
470+
const validTargetLangs = targetLangs as Language[];
433471

434472
const translationOptions: {
435473
sourceLang?: Language;
@@ -461,7 +499,7 @@ export class TranslateCommand {
461499

462500
const results = await this.translationService.translateToMultiple(
463501
text,
464-
targetLangs,
502+
validTargetLangs,
465503
{ ...translationOptions, skipCache: !options.cache }
466504
);
467505

@@ -488,6 +526,9 @@ export class TranslateCommand {
488526
throw new Error('Output directory is required for batch translation. Use --output <dir>');
489527
}
490528

529+
// Validate language code (Issue #3)
530+
this.validateLanguageCodes([options.to]);
531+
491532
// Build translation options
492533
const targetLang = options.to as Language;
493534
const translationOptions: {
@@ -576,6 +617,9 @@ export class TranslateCommand {
576617
* Used for small .txt, .md, .html, .srt, .xlf files
577618
*/
578619
private async translateTextFile(filePath: string, options: TranslateOptions): Promise<string> {
620+
// Validate language code (Issue #3)
621+
this.validateLanguageCodes([options.to]);
622+
579623
// Read file content
580624
const content = fs.readFileSync(filePath, 'utf-8');
581625

@@ -674,6 +718,9 @@ export class TranslateCommand {
674718
* Translate binary document (PDF, DOCX, etc.)
675719
*/
676720
private async translateDocument(filePath: string, options: TranslateOptions): Promise<string> {
721+
// Validate language code (Issue #3)
722+
this.validateLanguageCodes([options.to]);
723+
677724
const outputPath = options.output!;
678725

679726
// Build translation options

tests/unit/translate-command.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,4 +1509,82 @@ describe('TranslateCommand', () => {
15091509
);
15101510
});
15111511
});
1512+
1513+
describe('multi-language validation (Issue #3)', () => {
1514+
it('should reject invalid language codes in comma-separated list', async () => {
1515+
await expect(
1516+
translateCommand.translateText('Hello', {
1517+
to: 'es,invalid,fr',
1518+
})
1519+
).rejects.toThrow('Invalid target language code: "invalid"');
1520+
});
1521+
1522+
it('should accept all valid language codes', async () => {
1523+
(mockTranslationService.translateToMultiple as jest.Mock).mockResolvedValueOnce([
1524+
{ targetLang: 'es', text: 'Hola' },
1525+
{ targetLang: 'fr', text: 'Bonjour' },
1526+
{ targetLang: 'de', text: 'Hallo' },
1527+
]);
1528+
1529+
const result = await translateCommand.translateText('Hello', {
1530+
to: 'es,fr,de',
1531+
});
1532+
1533+
expect(result).toContain('Hola');
1534+
expect(result).toContain('Bonjour');
1535+
expect(result).toContain('Hallo');
1536+
expect(mockTranslationService.translateToMultiple).toHaveBeenCalledWith(
1537+
'Hello',
1538+
['es', 'fr', 'de'],
1539+
expect.any(Object)
1540+
);
1541+
});
1542+
1543+
it('should reject invalid single language code', async () => {
1544+
await expect(
1545+
translateCommand.translateText('Hello', {
1546+
to: 'invalid',
1547+
})
1548+
).rejects.toThrow('Invalid target language code: "invalid"');
1549+
});
1550+
1551+
it('should trim whitespace in language codes', async () => {
1552+
(mockTranslationService.translateToMultiple as jest.Mock).mockResolvedValueOnce([
1553+
{ targetLang: 'es', text: 'Hola' },
1554+
{ targetLang: 'fr', text: 'Bonjour' },
1555+
]);
1556+
1557+
const result = await translateCommand.translateText('Hello', {
1558+
to: ' es , fr ',
1559+
});
1560+
1561+
expect(result).toContain('Hola');
1562+
expect(mockTranslationService.translateToMultiple).toHaveBeenCalledWith(
1563+
'Hello',
1564+
['es', 'fr'],
1565+
expect.any(Object)
1566+
);
1567+
});
1568+
1569+
it('should reject empty language codes after trimming', async () => {
1570+
await expect(
1571+
translateCommand.translateText('Hello', {
1572+
to: 'es,,fr',
1573+
})
1574+
).rejects.toThrow('Invalid target language code: ""');
1575+
});
1576+
1577+
it('should validate language codes for file translation', async () => {
1578+
const fs = jest.requireActual('fs');
1579+
jest.spyOn(fs, 'statSync').mockReturnValue({ size: 1024, isDirectory: () => false } as any);
1580+
jest.spyOn(fs, 'readFileSync').mockReturnValue('Hello world');
1581+
1582+
await expect(
1583+
(translateCommand as any).translateFile('/input.txt', {
1584+
to: 'es,invalid,fr',
1585+
output: '/output',
1586+
})
1587+
).rejects.toThrow('Invalid target language code: "invalid"');
1588+
});
1589+
});
15121590
});

0 commit comments

Comments
 (0)