Skip to content

Commit f8caa56

Browse files
committed
fix(capabilities): thread experiment_id through is_stale
_load_fresh_analysis passes experiment_id to is_stale, which does not accept it, so every cache lookup that finds a stored block raises TypeError. The durable analyzer job burns all six attempts on it and the next poll enqueues a fresh one, so a task version that already carries a comparison can never regenerate -- the pane polls forever and renders it as an indefinite 'Analyzing agent behavior...' spinner. Seen in prod on scarf-cargotracker-quarkus-to-spring-migration v2 (four consecutive FAILED jobs, six attempts each) and 05-F3-poison-message- recurrence v23. The cache-lookup tests all stubbed the row out as None, which returns before the freshness comparison, so nothing covered the hit path.
1 parent 005002b commit f8caa56

2 files changed

Lines changed: 83 additions & 0 deletions

File tree

backend/api/services/agent_capabilities.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,7 @@ def is_stale(
468468
current_hash: str,
469469
schema_version: int,
470470
task_version_id: str,
471+
experiment_id: str | None = None,
471472
) -> bool:
472473
"""Whether a stored block must be regenerated. See ``stale_reason``."""
473474
return (
@@ -476,6 +477,7 @@ def is_stale(
476477
current_hash=current_hash,
477478
schema_version=schema_version,
478479
task_version_id=task_version_id,
480+
experiment_id=experiment_id,
479481
)
480482
is not None
481483
)

backend/tests/test_agent_capabilities_endpoint.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,87 @@ async def execute(statement):
220220
)
221221

222222

223+
async def _load_fresh_with_stored_block(block_metadata: dict, experiment_id):
224+
"""``_load_fresh_analysis`` against a session that returns one stored block.
225+
226+
Every other cache-lookup test hands back ``None``, which never reaches the
227+
freshness comparison -- the half where a hit is either served or rejected.
228+
"""
229+
230+
async def execute(statement):
231+
result = MagicMock()
232+
result.scalar_one_or_none.return_value = SimpleNamespace(
233+
block_metadata=block_metadata, output={"summary": "stored"}
234+
)
235+
return result
236+
237+
session = MagicMock()
238+
session.execute = execute
239+
240+
return await cc._load_fresh_analysis(
241+
session,
242+
task_id="t1",
243+
task_version_id="v1",
244+
current_hash="aaa",
245+
schema_version=1,
246+
experiment_id=experiment_id,
247+
)
248+
249+
250+
@pytest.mark.asyncio
251+
async def test_experiment_scoped_cache_hit_is_served():
252+
"""A stored block must survive the freshness check, not raise on it.
253+
254+
The scoping argument is threaded from the lookup into ``is_stale``; if the
255+
two signatures drift the call raises TypeError, the durable job burns all
256+
six attempts, and the pane polls a job that can never succeed -- rendered
257+
as an indefinite "Analyzing..." spinner rather than as an error.
258+
"""
259+
fresh = await _load_fresh_with_stored_block(
260+
{
261+
"cohort_hash": "aaa",
262+
"schema_version": 1,
263+
"task_version_id": "v1",
264+
"experiment_id": "e1",
265+
},
266+
experiment_id="e1",
267+
)
268+
269+
assert fresh == {"summary": "stored"}
270+
271+
272+
@pytest.mark.asyncio
273+
async def test_unscoped_cache_hit_is_served():
274+
fresh = await _load_fresh_with_stored_block(
275+
{
276+
"cohort_hash": "aaa",
277+
"schema_version": 1,
278+
"task_version_id": "v1",
279+
"experiment_id": None,
280+
},
281+
experiment_id=None,
282+
)
283+
284+
assert fresh == {"summary": "stored"}
285+
286+
287+
@pytest.mark.asyncio
288+
async def test_cache_hit_from_another_experiment_is_rejected():
289+
"""Scope is part of freshness: an experiment-scoped comparison covers a
290+
different trial set than the unscoped one and must not be served for it."""
291+
fresh = await _load_fresh_with_stored_block(
292+
{
293+
"cohort_hash": "aaa",
294+
"schema_version": 1,
295+
"task_version_id": "v1",
296+
"experiment_id": "e1",
297+
},
298+
experiment_id=None,
299+
)
300+
301+
assert fresh is None
302+
303+
223304
@pytest.mark.asyncio
224305
async def test_cache_lookup_filters_on_the_requested_version():
225306
"""The newest row for a task may belong to another version.

0 commit comments

Comments
 (0)