Skip to content

Commit 45fc5da

Browse files
Enfoirerclaude
andcommitted
refactor: address PR review comments (#203)
- Rename test method for clarity: test_skip_operation -> test_skip_test_operation - Simplify test class name: TestIssue203SaveFingerprintTestPromptHash -> TestIssue203TestPromptHashManagement - Consolidate duplicate read_fingerprint imports in _save_fingerprint_atomic Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 57cd218 commit 45fc5da

File tree

3 files changed

+9
-7
lines changed

3 files changed

+9
-7
lines changed

pdd/sync_orchestration.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,13 @@ def _save_fingerprint_atomic(basename: str, language: str, operation: str,
196196
model: The model used.
197197
atomic_state: Optional AtomicStateUpdate for atomic writes (Issue #159 fix).
198198
"""
199+
# Issue #203: Import read_fingerprint once for both branches
200+
from .sync_determine_operation import read_fingerprint
201+
199202
if atomic_state:
200203
# Buffer for atomic write
201204
from datetime import datetime, timezone
202-
from .sync_determine_operation import calculate_current_hashes, Fingerprint, read_fingerprint
205+
from .sync_determine_operation import calculate_current_hashes, Fingerprint
203206
from . import __version__
204207

205208
current_hashes = calculate_current_hashes(paths)
@@ -236,8 +239,7 @@ def _save_fingerprint_atomic(basename: str, language: str, operation: str,
236239
else:
237240
# Direct write using operation_log
238241
# Issue #203: Preserve test_prompt_hash from existing fingerprint for skip operations
239-
from .sync_determine_operation import read_fingerprint as read_fp
240-
existing_fp = read_fp(basename, language)
242+
existing_fp = read_fingerprint(basename, language)
241243
existing_test_prompt_hash = existing_fp.test_prompt_hash if existing_fp else None
242244
save_fingerprint(basename, language, operation, paths, cost, model,
243245
test_prompt_hash=existing_test_prompt_hash)

tests/test_operation_log.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ def test_fingerprint_hash_compatibility_with_sync(tmp_path):
630630
# ISSUE #203: test_prompt_hash auto-management in save_fingerprint
631631
# --------------------------------------------------------------------------------
632632

633-
class TestIssue203SaveFingerprintTestPromptHash:
633+
class TestIssue203TestPromptHashManagement:
634634
"""Test that save_fingerprint automatically manages test_prompt_hash based on operation type."""
635635

636636
def test_generate_operation_sets_test_prompt_hash_to_none(self, tmp_path):

tests/test_sync_orchestration.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4663,10 +4663,10 @@ def test_generate_then_test_workflow(self, tmp_path, monkeypatch):
46634663
fp_data3 = mock_atomic3.set_fingerprint.call_args[0][0]
46644664
assert fp_data3['test_prompt_hash'] == new_prompt_hash # Now linked to new prompt
46654665

4666-
def test_skip_operation_preserves_test_prompt_hash_without_atomic_state(self, tmp_path, monkeypatch):
4666+
def test_skip_test_operation_preserves_test_prompt_hash_without_atomic_state(self, tmp_path, monkeypatch):
46674667
"""
4668-
Issue #203 edge case: Skip operations (without atomic_state) should preserve
4669-
existing test_prompt_hash instead of losing it.
4668+
Issue #203 edge case: Skip-prefixed operations (like skip:test, without atomic_state)
4669+
should preserve existing test_prompt_hash instead of losing it.
46704670
"""
46714671
import json
46724672
from pdd.sync_orchestration import _save_fingerprint_atomic, META_DIR

0 commit comments

Comments
 (0)