runtime: the sync deadline guard lets a node that overran commit its writes when the timer never fired

[Shashankss1205]

Summary

The sync deadline_guard's exit path only raises when its interrupt actually fired:

    try:
        try:
            yield
        finally:
            disarm()
    except NodeDeadlineExceeded as exc:
        raise NodeDeadlineExceeded(detail()) from exc
    if state["fired"]:
        raise NodeDeadlineExceeded(detail())

The async guard performs one more check at the same boundary, and its comment names exactly the case the sync guard misses (grapharc/runtime/graph.py:255-260):

    # Reached only when the node returned normally. It may have swallowed the
    # cancellation, or the deadline may have passed without the timer running
    # yet; either way its writes must not land.
    left = meter.remaining_seconds()
    if state["fired"] or (left is not None and left <= 0):
        raise NodeDeadlineExceeded(detail())

On the fallback mechanism (threading.Timer + PyThreadState_SetAsyncExc — the one every worker-thread run uses), the timer thread needs the GIL to run fire(). A node that holds the GIL through the deadline (a long C-level call: a big regex match, a GIL-holding extension — or simply timer scheduling latency) returns before fire() ever runs; disarm() then sets armed=False, the pending timer is cancelled, state["fired"] is False, and the guard raises nothing. Verified against the checked-out code:

result: {'out': {'out': 'committed after 0.60s'}}
meter snapshot: {'iterations': 1, 'tokens': 0, 'elapsed_seconds': 0.603}

— a run under Budget(max_seconds=0.2) whose only node ran 0.6s, committed its writes, and invoke() returned success. No BudgetExceeded was raised anywhere.

Why this matters

README states the boundary guarantee unconditionally: "Even then the deadline holds at the node boundary: a node that overran does not get its writes into state." The guard's own docstring promises the same ("Short of that last case the ceiling is honoured at the node boundary"), and its stated degraded mode for blocking C calls is "raises on return" — but nothing raises when the timer never got to fire. The gap applies to the runs the docstring itself flags as fallback-only: "any run driven from a worker thread — every request handler in a threaded server, every ThreadPoolExecutor caller — falls back to mechanism 2 for the whole run". For a mid-graph node the next node's _enter budget check stops the run (but the overrunning node's writes have already landed); for the last node there is no next check at all, so the run completes past its wall-clock ceiling and reports success. The async guard already refuses this exact case, so sync and async nodes currently honour different contracts.

Where in the code

• grapharc/runtime/budget.py:489-490 — the exit check tests only state["fired"]
• grapharc/runtime/graph.py:255-260 — the async guard's exit check to mirror (state["fired"] or left <= 0)
• grapharc/runtime/budget.py:385-388 — the docstring promise the current code does not keep
• README.md — "Budgets" paragraph: "a node that overran does not get its writes into state"

uv run python - <<'EOF'
import sys, threading, time
from pydantic import BaseModel
from grapharc.runtime.graph import GraphARC, END, START
from grapharc.runtime.budget import Budget

class S(BaseModel):
    out: str = ""

g = GraphARC(S, name="demo")
def slow(state):
    old = sys.getswitchinterval()
    sys.setswitchinterval(5)  # stand-in for any GIL-holding C call
    try:
        t0 = time.monotonic()
        while time.monotonic() - t0 < 0.6:
            pass
    finally:
        sys.setswitchinterval(old)
    return {"out": "committed"}
g.add_node("slow", slow, writes={"out"})
g.add_edge(START, "slow"); g.add_edge("slow", END)
compiled = g.compile()

result = {}
def drive():  # worker thread => mechanism 2, like any threaded server
    try:
        result["out"] = compiled.invoke({}, budget=Budget(max_seconds=0.2))
    except BaseException as e:
        result["exc"] = repr(e)
t = threading.Thread(target=drive); t.start(); t.join()
print(result)  # {'out': ...} today; must be {'exc': NodeDeadlineExceeded(...)}
EOF

What to change

1. Mirror the async guard in deadline_guard's exit path: after disarm(), raise NodeDeadlineExceeded(detail()) when state["fired"] or meter.remaining_seconds() is not None and <= 0.
2. Update the guard's "What this does not guarantee" docstring: the C-call caveat becomes "the node is not interrupted mid-call, but the guard still raises on exit", matching what the async guard already documents.
3. Add a deterministic regression test: run deadline_guard on a worker thread with threading.Timer monkeypatched to a timer that never fires (or the switch-interval repro above), have the body outlast max_seconds, and assert the guard raises on exit. That tests the exit contract directly rather than racing the timer thread.

Out of scope: the documented never-returns case (a node swallowing every interrupt in a while True cannot be stopped, and no exit check runs because the node never exits); the SIGALRM mechanism (pending signals are processed before the node can return, so the gap is not reachable there); and _async_deadline, which is already correct.

How to verify

uv run pytest -q
uv run ruff check .

The new test in tests/test_budget_enforcement.py (e.g. test_an_overrun_is_refused_at_exit_even_if_the_timer_never_fired) must fail on the current code — today the guard exits cleanly and the writes land — and pass with the fix. The existing swallower tests (test_the_interrupt_is_re_armed_after_a_node_swallows_it and the worker-thread variant) must pass unchanged.

Acceptance criteria

• A sync node that returns after max_seconds raises NodeDeadlineExceeded at the guard's exit even when no interrupt was delivered, so its writes never reach state
• The repro above prints an exception instead of a committed result
• Existing deadline tests (swallower, re-arm, refuses-to-start-past-deadline) pass unchanged
• README's "a node that overran does not get its writes into state" is accurate again (or reworded if any residual caveat remains), and the guard docstring matches the behaviour
• uv run pytest -q green, uv run ruff check . clean

Skill level — experience required

The fix itself is two lines, but reviewing it needs a working model of asynchronous exceptions, GIL scheduling, and why the timer can lose the race — and the regression test must be deterministic rather than a coin toss (this repo has already burned once on asserting where CPython delivers async exceptions; see the comments in tests/test_budget_enforcement.py:462-505). Someone comfortable with PyThreadState_SetAsyncExc semantics and the existing test patterns should take this.