Skip to content

Commit 3e85268

Browse files
Serhan-Asadclaude
andauthored
test: add failing tests for cost extraction wrong index (#555) (#556)
* fix: extract cost/model from correct tuple index per operation (#555) The sync loop read result[1] as cost for all operations, but each returns a different tuple format. generate's result[1] is a bool (was_incremental) which passed isinstance(bool, (int, float)) and was counted as $1.00; crash/fix/verify have cost at index 4, not 1, so their costs were always $0.00. Two helpers now dispatch on the operation name to read the correct index. Fixes #555 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: apply cost extraction fix to operation_log.py and add passing tests (#555) - operation_log.py: use _extract_cost_from_result/_extract_model_from_result helpers instead of hardcoded result[1]/result[2] (same bug as sync_orchestration.py) - Update test files to import and test the actual fixed helpers (29/29 pass) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f9df4b8 commit 3e85268

File tree

4 files changed

+884
-15
lines changed

4 files changed

+884
-15
lines changed

pdd/operation_log.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,8 +336,9 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
336336
model = "unknown"
337337
if success and result:
338338
if isinstance(result, tuple) and len(result) >= 3:
339-
if isinstance(result[1], (int, float)): cost = float(result[1])
340-
if isinstance(result[2], str): model = str(result[2])
339+
from .sync_orchestration import _extract_cost_from_result, _extract_model_from_result
340+
cost = _extract_cost_from_result(operation, result)
341+
model = _extract_model_from_result(operation, result)
341342

342343
update_log_entry(entry, success=success, cost=cost, model=model, duration=duration, error=error_msg)
343344
if basename and language:

pdd/sync_orchestration.py

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,46 @@
7070
# Note: _safe_basename is imported from sync_determine_operation
7171

7272

73+
def _extract_cost_from_result(operation: str, result: tuple) -> float:
74+
"""Extract cost from an operation result tuple at the correct index.
75+
76+
Each operation returns a different tuple format:
77+
- crash/fix/verify: 6-tuple (..., cost, model) — cost at index 4
78+
- generate: 4-tuple (content, was_incremental, cost, model) — cost at index 2
79+
- example/test/test_extend/auto-deps: 3-or-4-tuple (..., cost, model, ...) — cost at index 1
80+
"""
81+
if operation in ('crash', 'fix', 'verify'):
82+
idx = 4
83+
elif operation == 'generate':
84+
idx = 2
85+
else:
86+
idx = 1
87+
if len(result) > idx:
88+
val = result[idx]
89+
if isinstance(val, (int, float)) and not isinstance(val, bool):
90+
return float(val)
91+
return 0.0
92+
93+
94+
def _extract_model_from_result(operation: str, result: tuple) -> str:
95+
"""Extract model name from an operation result tuple at the correct index.
96+
97+
Each operation returns a different tuple format:
98+
- crash/fix/verify: 6-tuple (..., cost, model) — model at index 5
99+
- generate: 4-tuple (content, was_incremental, cost, model) — model at index 3
100+
- example/test/test_extend/auto-deps: 3-or-4-tuple (..., cost, model, ...) — model at index 2
101+
"""
102+
if operation in ('crash', 'fix', 'verify'):
103+
idx = 5
104+
elif operation == 'generate':
105+
idx = 3
106+
else:
107+
idx = 2
108+
if len(result) > idx and isinstance(result[idx], str):
109+
return result[idx]
110+
return 'unknown'
111+
112+
73113
def _use_agentic_path(language: str, agentic_mode: bool) -> bool:
74114
"""Returns True if we should use agentic path (non-Python OR agentic_mode for Python).
75115
@@ -1854,8 +1894,7 @@ def __init__(self, rc, out, err):
18541894
success = pdd_files['test'].exists()
18551895
else:
18561896
success = bool(result[0])
1857-
# Cost is always at index 1 in both 3-tuple and 4-tuple returns
1858-
cost = result[1] if len(result) >= 2 and isinstance(result[1], (int, float)) else 0.0
1897+
cost = _extract_cost_from_result(operation, result)
18591898
current_cost_ref[0] += cost
18601899
else:
18611900
success = result is not None
@@ -1877,17 +1916,8 @@ def __init__(self, rc, out, err):
18771916
actual_cost = result.get('cost', 0.0)
18781917
model_name = result.get('model', 'unknown')
18791918
elif isinstance(result, tuple) and len(result) >= 3:
1880-
# cmd_test_main returns 4-tuple: (content, cost, model, agentic_success)
1881-
# Other commands may return either:
1882-
# - 3-tuple: (content, cost, model), e.g. some context operations
1883-
# - 4-tuple: (content, was_incremental, cost, model_name), e.g. code_generator_main
1884-
# For tests, cost is at index 1; for most other 4+ tuples, cost is at index -2 and model at -1.
1885-
if operation in ('test', 'test_extend') and len(result) >= 4:
1886-
actual_cost = result[1] if isinstance(result[1], (int, float)) else 0.0
1887-
model_name = result[2] if isinstance(result[2], str) else 'unknown'
1888-
else:
1889-
actual_cost = result[1] if isinstance(result[1], (int, float)) else 0.0
1890-
model_name = result[2] if len(result) >= 3 and isinstance(result[2], str) else 'unknown'
1919+
actual_cost = _extract_cost_from_result(operation, result)
1920+
model_name = _extract_model_from_result(operation, result)
18911921
last_model_name = str(model_name)
18921922
operations_completed.append(operation)
18931923
_save_fingerprint_atomic(basename, language, operation, pdd_files, actual_cost, str(model_name), atomic_state=atomic_state)

0 commit comments

Comments
 (0)