docs(cache): document deterministic cache key generation

Added comprehensive documentation and test to verify cache key generation
is deterministic regardless of option order.

- Added test verifying identical cache keys for different option orders
- Documented intentional property ordering in generateCacheKey() method
- Added explanatory comments explaining why property order matters
- Prevents future developers from accidentally breaking cache key generation

**Implementation Note**: The current implementation already handles this
correctly by creating a new object with fixed property order. This commit
documents the intentional design to prevent accidental cache key breakage.

**Impact**: Ensures consistent cache behavior and prevents reduced cache
hit rates from property ordering changes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: sjsyrek

CHANGELOG.md

@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+- **Cache Key Determinism Documentation** - Documented intentional property ordering for cache keys
+  - Added comprehensive test to verify cache keys are identical regardless of option order
+  - Added detailed documentation explaining why property order matters in `generateCacheKey()`
+  - Property order in cache data object is now explicitly documented as intentional
+  - Prevents accidental cache key breakage from property reordering
+  - **Impact**: Ensures future developers understand the importance of property order
+  - Location: `src/services/translation.ts:386-417`, test added to `tests/unit/translation-service.test.ts`
+
 ### Changed
 - **Test Coverage Enhancement** - Comprehensive integration and E2E test expansion
   - Created 10 new test files covering critical workflows and CLI behavior

src/services/translation.ts

@@ -385,20 +385,29 @@ export class TranslationService {
 
   /**
    * Generate cache key from text and options
+   *
+   * IMPORTANT: This method creates a new object with properties in a FIXED order
+   * to ensure deterministic cache keys. Two translation requests with identical
+   * parameters must generate the same cache key, regardless of the order in which
+   * properties were specified in the input options object.
+   *
+   * The property order in `cacheData` is intentional and must not be changed,
+   * as it directly affects cache key generation via JSON.stringify().
    */
   private generateCacheKey(text: string, options: TranslationOptions): string {
-    // Create a stable representation of the translation request
+    // Create a stable representation with deterministic property order
+    // Property order matters because JSON.stringify() preserves insertion order
     const cacheData = {
-      text,
-      targetLang: options.targetLang,
-      sourceLang: options.sourceLang,
-      formality: options.formality,
-      glossaryId: options.glossaryId,
-      context: options.context,
-      // Note: preserveFormatting doesn't affect translation output caching
+      text,                      // 1. Text to translate
+      targetLang: options.targetLang,    // 2. Target language
+      sourceLang: options.sourceLang,    // 3. Source language (if specified)
+      formality: options.formality,      // 4. Formality level
+      glossaryId: options.glossaryId,    // 5. Glossary ID
+      context: options.context,          // 6. Context hint
+      // Note: preserveFormatting doesn't affect translation output, so not cached
     };
 
-    // Generate SHA-256 hash
+    // Generate SHA-256 hash of the stable representation
     const hash = crypto
       .createHash('sha256')
       .update(JSON.stringify(cacheData))

tests/unit/translation-service.test.ts

@@ -907,5 +907,80 @@ describe('TranslationService', () => {
       expect(mockCacheService.set).not.toHaveBeenCalled();
       expect(mockDeepLClient.translate).toHaveBeenCalledTimes(1);
     });
+
+    it('should generate deterministic cache keys regardless of option order (BUG #1)', async () => {
+      // This test demonstrates that cache keys must be deterministic
+      // Two requests with identical options but different ordering should use the same cache key
+
+      // Reset mocks to ensure clean state
+      mockCacheService.get.mockReturnValue(null); // No cache hits
+      mockDeepLClient.translate.mockResolvedValue({
+        text: 'Hola',
+      });
+
+      // First call with options in one order
+      await translationService.translate('Hello', {
+        targetLang: 'es',
+        sourceLang: 'en',
+        formality: 'more',
+        glossaryId: 'glossary-123',
+        context: 'greeting',
+      });
+
+      // Second call with identical options but different ordering
+      await translationService.translate('Hello', {
+        context: 'greeting',
+        glossaryId: 'glossary-123',
+        formality: 'more',
+        sourceLang: 'en',
+        targetLang: 'es',
+      });
+
+      // Verify the cache keys are the same by checking cache.set calls
+      const cacheSetCalls = mockCacheService.set.mock.calls;
+      expect(cacheSetCalls.length).toBe(2); // Two calls, should generate same key
+
+      // Extract the cache keys (first parameter of each set call)
+      const key1 = cacheSetCalls[0]?.[0] as string;
+      const key2 = cacheSetCalls[1]?.[0] as string;
+
+      // The cache keys MUST be identical despite different option ordering
+      // This is critical for cache hit rate - identical requests should use the same key
+      expect(key1).toBe(key2);
+    });
+
+    it('should use cached result when options are provided in different order', async () => {
+      // Set up cache to return a hit for the second call
+      let callCount = 0;
+      mockCacheService.get.mockImplementation(() => {
+        callCount++;
+        if (callCount === 1) {
+          return null; // First call: miss
+        }
+        return { text: 'Hola (cached)' }; // Second call: hit
+      });
+
+      mockDeepLClient.translate.mockResolvedValue({
+        text: 'Hola (fresh)',
+      });
+
+      // First call
+      const result1 = await translationService.translate('Hello', {
+        targetLang: 'es',
+        formality: 'more',
+      });
+      expect(result1.text).toBe('Hola (fresh)');
+      expect(mockDeepLClient.translate).toHaveBeenCalledTimes(1);
+
+      // Second call with same options but different order
+      const result2 = await translationService.translate('Hello', {
+        formality: 'more',
+        targetLang: 'es',
+      });
+
+      // Should use cached result
+      expect(result2.text).toBe('Hola (cached)');
+      expect(mockDeepLClient.translate).toHaveBeenCalledTimes(1); // Still only 1 API call
+    });
   });
 });