Skip to content

Commit 130ff53

Browse files
uipreligaclaude
andcommitted
fix(timing): claude-code's windows tile across a tool result
`on_user_message` reset the generation mark to `self.clock.now()`, so the next window opened when the tool RESULT arrived instead of tiling from the previous emission's close. Everything in between — SDK transport, CLI processing, next-request dispatch — fell into no bucket at all, and the four-bucket identity stopped closing. The live stream delivers TWO user messages per tool call, and the mark was reset on each, so the window opened at the LAST one. Traced on `tasks/dataset_example.yaml`: msg2 (issues the Write) closes 27.773889 -> 28.067824 user message #1 28.080804 <- mark reset here user message #2 30.009645 <- and again, 1.93s later msg3 window opened at 30.009645 (should be 28.067824) 1.94 s lost from an 11.7 s turn. CI's residual gate has been failing on exactly these two rows (16.668% and 16.325% on the runner, 21-28% locally). Same task after the fix: 0.02%, worst turn 4.5 ms — the known `turn_start_time`-vs-`AgentStartEvent` baseline and nothing else. The tool's own interval is not double-counted: it is a separate bucket and `subtract_tool_time` clips the tool union out of every window it overlaps, once, for all five harnesses. That central subtraction is precisely what lets the reducer leave its mark alone — the same rule pi follows with `gen_mark`, and the one pi was explicitly fixed for. WHY THIS SURVIVED, which is worth more than the one-line fix. Three ways to write a test for it cannot fail, and I wrote two of them before getting one that does: 1. A tool-heavy shape. Three concurrent `sleep 3` calls make the tool union absorb the interval; every live probe I ran read 0.05% and I concluded the harness was healthy. The defect needs a FAST tool. 2. A single tool result. claude-code reconstructs `execution_started_at` by subtracting the measured duration from the resolve instant, so with one message the discarded interval and the tool's own span are the SAME milliseconds — `subtract_tool_time` removes them either way and the identity closes with or without the bug. This is why the existing `_claude_turn` case passed throughout. 3. A duplicate tool RESULT as the second message. That re-resolves the call and stretches the tool span over the very interval being probed. `test_a_slow_tool_result_round_trip_is_not_lost` scripts the shape that discriminates: a 20 ms tool, then a second user message carrying no tool result 2 s later. Mutation-checked — restoring the reset fails that case by exactly -2000 ms and leaves the other seven green, which is the production situation reproduced in the suite. No golden regeneration: `_scrub.py::SCRUB_KEYS` masks every timing value, so the corpus cannot see this class of change. That is a known property of the goldens, not an oversight, and it is the reason the ms-exact identity contract exists alongside them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpDo37ypvLjLiWXFsEkg6k
1 parent 9202699 commit 130ff53

3 files changed

Lines changed: 158 additions & 6 deletions

File tree

‎docs/agents/HARNESS_PARITY.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,29 @@ figures in the table above are means of six live `tasks/hello_date` turns per
262262
harness and move with CLI cache warmth, so read their ORDER OF MAGNITUDE, not
263263
the digits.
264264

265+
**Windows tile ACROSS a tool result, on every harness.** claude-code used to
266+
reset its generation mark when the tool-result `UserMessage` arrived, so the
267+
next window opened at the result rather than tiling from the previous
268+
emission. Everything in between — SDK transport, CLI processing, next-request
269+
dispatch — fell into no bucket. The live stream delivers TWO user messages per
270+
tool call, ~2 s apart, and the mark was reset on each, so the window opened at
271+
the LAST one: measured on `tasks/dataset_example.yaml` at 1.94 s lost from an
272+
11.7 s turn, 16-28% of wall clock, and it is what failed CI's residual gate.
273+
The mark is now left where `on_assistant_message` put it and the tool's own
274+
interval is removed centrally by `subtract_tool_time`, exactly as pi does with
275+
`gen_mark`. Same task after the fix: 0.02%.
276+
277+
Why it survived so long is the more useful half. A tool-heavy shape cannot see
278+
it — three concurrent `sleep 3` calls make the tool union absorb the interval
279+
and the residual reads 0.05%. Neither can a single-tool-result fixture:
280+
claude-code reconstructs `execution_started_at` by subtracting the measured
281+
duration from the resolve instant, so with one message the discarded interval
282+
and the tool's own span are the SAME milliseconds and the identity closes
283+
either way. It takes a FAST tool plus a SECOND user message carrying no tool
284+
result to separate them, which is what
285+
`test_a_slow_tool_result_round_trip_is_not_lost` scripts. Two earlier drafts of
286+
that test could not fail.
287+
265288
**Four turn buckets, two task buckets — and they are different scopes.** The
266289
four above tile ONE TURN and their identity (`head + Σgeneration + UNION(tool)
267290
+ tail == the turn's span`) is asserted to the millisecond by

‎src/coder_eval/agents/claude_code_agent.py‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,28 @@ def on_user_message(self, message: Message) -> None:
597597
"""Process tool results (and a sub-agent's terminal generation) from a
598598
tool-result UserMessage. The sub-agent message is appended BEFORE the
599599
tool-result loop — its position in ``sdk_messages`` is observable."""
600-
self.last_event_wall = self.clock.now()
600+
# The generation mark is DELIBERATELY NOT advanced here. It used to be
601+
# reset to `self.clock.now()`, which opened the next window at the
602+
# instant the tool RESULT arrived rather than tiling it from the
603+
# previous window's close — so everything between the tool finishing
604+
# and its result reaching this handler (SDK transport, CLI processing,
605+
# next-request dispatch) fell into no bucket at all. Measured on
606+
# `tasks/dataset_example.yaml`: a 21.5 ms `Write` followed by a 2511.7 ms
607+
# round trip, which is 21% of an 11.7 s turn accounted to nothing and
608+
# the reason CI's residual gate failed on that task while a
609+
# `sleep`-heavy probe read 0.05%. A tool-heavy shape cannot see this:
610+
# the tool union absorbs the interval. A fast tool leaves it exposed.
611+
#
612+
# Leaving the mark where `on_assistant_message` put it makes the next
613+
# window run from the previous emission's arrival, so the windows tile
614+
# the turn contiguously — the same rule pi follows with `gen_mark`, and
615+
# the one pi was explicitly fixed for.
616+
#
617+
# The tool's OWN interval is not double-counted by this: it is a
618+
# separate bucket, and `streaming/collector.py::subtract_tool_time`
619+
# clips the tool union out of every window it overlaps, once, for all
620+
# five harnesses. That is exactly why the mark can be left alone here —
621+
# the reducer no longer has to carve the tool out of its own windows.
601622

602623
sub_msg = self._agent._synthesize_subagent_terminal_message(message, self.sdk_model_used)
603624
if sub_msg is not None:

‎tests/test_timing_identity_contract.py‎

Lines changed: 113 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -449,11 +449,17 @@ def _claude_turn(monkeypatch: pytest.MonkeyPatch) -> Turn:
449449
`tests/test_agent_telemetry.py`; here it shows up as the windows still
450450
tiling.
451451
452-
Note where its windows do NOT tile: the tool result resets both marks, so
453-
the interval between the emission that ISSUED the call and the result is
454-
left outside every window. That gap is the tool's own execution, which is
455-
exactly what the tool bucket claims — which is why the identity still
456-
closes to the millisecond.
452+
Its windows TILE across the tool result, and this case only proved that by
453+
accident until the reducer was fixed. The mark used to be reset when the
454+
result arrived, so the interval between the emission that ISSUED the call
455+
and the result landed in no bucket. Here that interval IS the tool's
456+
execution exactly — the case scripts the result at the instant the tool
457+
ends — so the tool bucket happened to claim the same milliseconds and the
458+
identity closed anyway. On a real turn the two differ: a 21.5 ms `Write`
459+
can be followed by a 2.5 s round trip, and 21% of the turn goes missing.
460+
`test_a_slow_tool_result_round_trip_is_not_lost` is the case that
461+
discriminates; this one deliberately keeps the coincident shape so the two
462+
read as a pair.
457463
"""
458464
from coder_eval.agents import claude_code_agent as claude_module
459465
from coder_eval.agents.claude_code_agent import ClaudeCodeAgent, _ClaudeTurnState
@@ -517,6 +523,98 @@ def _monotonic() -> float:
517523
return Turn(started_ms=0.0, ended_ms=3000.0, messages=list(state.sdk_messages), commands=commands)
518524

519525

526+
def _claude_slow_result_turn(monkeypatch: pytest.MonkeyPatch) -> Turn:
527+
"""A FAST tool followed by a SLOW result round trip — the shape that hid a defect.
528+
529+
``_claude_turn`` above scripts the tool result at the instant the tool
530+
finishes, so the un-tiled interval and the tool's own span were the same
531+
milliseconds and the identity closed even while the mark was being reset.
532+
Every live probe had the same blind spot from the other direction: three
533+
concurrent ``sleep 3`` calls make the tool union so large that the round
534+
trip rounds away (measured: 0.05% residual).
535+
536+
Here the tool runs for 20 ms and its result takes 2000 ms to come back,
537+
which is `tasks/dataset_example.yaml` — the task CI actually runs, where a
538+
21.5 ms ``Write`` met a 2511.7 ms round trip and 21% of the turn was
539+
accounted to nothing. The identity closing here is the whole point: the
540+
window after the result must tile from the previous emission, not open
541+
when the result lands.
542+
"""
543+
from coder_eval.agents import claude_code_agent as claude_module
544+
from coder_eval.agents.claude_code_agent import ClaudeCodeAgent, _ClaudeTurnState
545+
from coder_eval.streaming.events import AgentEndStatus as _AgentEndStatus
546+
from tests._fixtures.golden_streams.claude_fixtures import AssistantMessage as SdkAssistantMessage
547+
from tests._fixtures.golden_streams.claude_fixtures import ToolUseBlock, UserMessage, message_start
548+
549+
clock = _InjectedClock()
550+
551+
def _monotonic() -> float:
552+
return clock.at_ms / 1000.0
553+
554+
monkeypatch.setattr(claude_module, "time", SimpleNamespace(monotonic=_monotonic))
555+
556+
agent = ClaudeCodeAgent(parse_agent_config(type=AgentKind.CLAUDE_CODE, permission_mode="acceptEdits"))
557+
collector = EventCollector()
558+
commands: list[CommandTelemetry] = []
559+
560+
clock.at_ms = 200
561+
state = _ClaudeTurnState(
562+
agent,
563+
emit=CompositeStreamCallback(
564+
[
565+
collector,
566+
SimpleNamespace(on_event=lambda e: commands.append(e.tool) if isinstance(e, ToolEndEvent) else None),
567+
]
568+
),
569+
collector=collector,
570+
task_id="t",
571+
user_input="go",
572+
iteration=1,
573+
max_turns=None,
574+
log=agent._log,
575+
turn_start_time=_monotonic(),
576+
deadline=None,
577+
clock=clock,
578+
)
579+
580+
clock.at_ms = 500
581+
state.on_stream_event(message_start("m1"))
582+
clock.at_ms = 980
583+
state.on_assistant_message(
584+
SdkAssistantMessage(
585+
[ToolUseBlock("c1", "Write", {"file_path": "out.txt"})],
586+
usage={"input_tokens": 10, "output_tokens": 5},
587+
message_id="m1",
588+
)
589+
)
590+
# The tool itself is 20 ms. What follows is the shape a live turn actually
591+
# has, traced off `tasks/dataset_example.yaml`: the SDK delivers TWO user
592+
# messages, the second ~2 s after the first. The old code reset the mark on
593+
# each, so the next window opened at the LAST one and that 2 s vanished.
594+
#
595+
# One user message is not enough to catch it, and that is exactly why this
596+
# shipped: claude-code reconstructs `execution_started_at` by subtracting
597+
# the measured duration from the resolve instant, so with a single message
598+
# the discarded interval and the tool's own span are the SAME milliseconds
599+
# — `subtract_tool_time` removes them either way and the identity closes
600+
# with or without the bug. The second message is what separates them.
601+
clock.at_ms = 1000
602+
state.on_user_message(UserMessage("c1", False, "written"))
603+
clock.at_ms = 3000
604+
# The second one carries NO tool-result block, which is what the live
605+
# stream does — the traced turn kept its 21.5 ms Write span across it. A
606+
# duplicate RESULT would instead re-resolve the call and stretch the tool
607+
# span over the very interval this case exists to expose, which is a third
608+
# way to write a test that cannot fail.
609+
state.on_user_message(SimpleNamespace(content=[], tool_use_result=None))
610+
state.on_stream_event(message_start("m2"))
611+
clock.at_ms = 3400
612+
state.on_assistant_message(SdkAssistantMessage([], usage={"input_tokens": 10, "output_tokens": 5}, message_id="m2"))
613+
state.finalize(_AgentEndStatus.COMPLETED)
614+
615+
return Turn(started_ms=0.0, ended_ms=3800.0, messages=list(state.sdk_messages), commands=commands)
616+
617+
520618
# --------------------------------------------------------------------------
521619
# The contract
522620
# --------------------------------------------------------------------------
@@ -542,6 +640,16 @@ def test_claude_code_buckets_tile_the_turn(monkeypatch: pytest.MonkeyPatch):
542640
assert_identity_closes(_claude_turn(monkeypatch))
543641

544642

643+
def test_a_slow_tool_result_round_trip_is_not_lost(monkeypatch: pytest.MonkeyPatch):
644+
"""The discriminating case: a fast tool whose result takes 2 s to come back.
645+
646+
Reverting the fix (re-adding `last_event_wall = self.clock.now()` to
647+
`on_user_message`) fails THIS and leaves every other case in the file
648+
green, which is exactly what happened in production.
649+
"""
650+
assert_identity_closes(_claude_slow_result_turn(monkeypatch))
651+
652+
545653
def test_every_built_in_harness_has_a_case():
546654
"""A sensor that silently covers four of five is worse than one naming the gap.
547655

0 commit comments

Comments
 (0)