Skip to content

Give the eager-transport test fake the capture-schedule gate - #147

Merged
zaoxing merged 3 commits into
mainfrom
fix/stale-eager-transport-fake
Sep 24, 2026
Merged

zaoxing merged 3 commits into
mainfrom
fix/stale-eager-transport-fake

Conversation

@zaoxing

@zaoxing zaoxing commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Five GPU tests in tests/test_producer_chunked_schema.py have failed on main since #122, all with the same error:

AttributeError: '_FakeEagerTransport' object has no attribute 'capture_step'

Why. #122 (DMI-configurator) added a capture-schedule gate to HookPoint.forward that reads transport.capture_step on the active transport (src/dmi/hooks/point.py:266-268). The real RingTransport sets capture_step = True (ring.py:244), so production is unaffected. This test file's own _FakeEagerTransport never gained the attribute, so the eager-path tests raised before reaching the capacity and staging behaviour they pin. They're gpu-marked and CI has no GPU runner, so nothing flagged it.

Fix. One attribute on the fake, capture_step = True, matching the real transport's default.

Evidence (RTX 4090): 5 failed / 15 passed before; 20 passed after.

Independent of the #143 stack.

Since the independent review (2026-09-24)

An independent review judged this "ship" and suggested hardening; both suggestions are done (d9165fa, edd8af0):

  • The tests now build a real RingTransport over the fake engine, the way tests/test_hook_point_eager_cap_cache.py does, and _FakeEagerTransport is deleted. A hand-copied transport is what let DMI-configurator: structured configuration, YAML, and web UI #122's new attribute break these tests unnoticed; it also reimplemented effective_cap rather than using the real property.
  • A new test pins the capture-schedule gate on the eager path: with capture_step=False and force_eager=True, a hook dispatches nothing (no flush, reserve, CPU-direct or producer call). With the gate moved below the eager block, it fails.

Evidence (RTX 4090): 21 passed. CI green at edd8af0.

#122 added a capture-schedule gate to HookPoint.forward: it reads
transport.capture_step on the active transport before anything else. The
real RingTransport sets it to True. This file's _FakeEagerTransport never
gained the attribute, so all five eager-path tests have raised
AttributeError at hooks/point.py since #122, before reaching the capacity
and staging behaviour they exist to pin. They are marked gpu, and CI has no
GPU runner, so nothing flagged it.

The fake now arms the gate the way the real transport does.

GPU (RTX 4090): 5 failed, 15 passed before; 20 passed after.

Claude-Session: https://claude.ai/code/session_01PcY9QS1FAehHTzkjdpvN6Y
Copilot AI lite review requested due to automatic review settings September 23, 2026 20:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@zaoxing
zaoxing requested review from Samfisheryu and a lite review from Copilot September 23, 2026 20:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

_FakeEagerTransport was a hand-copied RingTransport, and it drifted: it
lacked capture_step until the previous commit, so five GPU tests raised
AttributeError after #122 added the capture-schedule gate. It also
reimplemented production logic by precomputing effective_cap instead of
going through the transport's cached property.

The tests now build a real RingTransport(engine) over the fake engine,
set force_eager, and replace only submit_cpu_direct with a list append.
tests/test_hook_point_eager_cap_cache.py already does this. The fake
engine gains the payload_tensor() the constructor needs, and each hook
writes into that payload. Any attribute RingTransport gains in future is
therefore present in these tests, and effective_cap is the production
property. Every test's assertions are unchanged, and the now-unused
effective_ring_bytes import is gone.

GPU (RTX 4090, svc worktree, point.py and ring.py identical to
origin/main): 20 passed.

Claude-Session: https://claude.ai/code/session_01PcY9QS1FAehHTzkjdpvN6Y
Nothing covered #122's capture-schedule gate on the eager path. When the
schedule refuses a step, the driver skips plan/commit, so no ring space
is reserved and no meta is pushed. If a producer still ran, it would
write unreserved bytes and desync the task/meta FIFO. HookPoint.forward
checks transport.capture_step after the `if not self.enabled` early
return and before the CUDA probe, so a refused step never reaches the
force_eager block.

The new test arms a real RingTransport with force_eager=True and
capture_step=False. It then calls the hook with one tensor over staging
(flush + cpu_direct) and one that fits staging but not the current slack
(flush + reserve + ring). It asserts that nothing was flushed, reserved,
submitted to cpu_direct or dispatched to a producer.

GPU (RTX 4090, svc worktree, point.py and ring.py identical to
origin/main): 21 passed. On a scratch copy of the package with the gate
moved below the eager block, the new test fails with flushes == 2, and
the four counters read flushes=2 reserved=[32] direct=1 dispatched=1.
The other 20 tests still pass there.

Claude-Session: https://claude.ai/code/session_01PcY9QS1FAehHTzkjdpvN6Y
@zaoxing
zaoxing merged commit 439e914 into main Sep 24, 2026
2 checks passed
@zaoxing
zaoxing deleted the fix/stale-eager-transport-fake branch September 24, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants