Collect model input after cadence pacing

[ZenAlexa]

Summary

I collect input after cadence pacing so events received during the wait reach the upcoming model step. Reset also clears buffered step timing at that final input snapshot.

Addresses #543.

Validation

• Deterministic regressions cover input received during pacing and reset before and during pacing.
• V2 CPU suite: 210 passed, 3 deselected.
• Ruff formatting, focused lint, and diff checks passed.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[Copilot]

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

[greptile-apps]

Confidence Score: 2/5

The PR does not appear safe to merge because three previously reported message-settlement paths can still silently discard accepted model-loop operations.

Findings

1.  Post-cadence messages are dropped ▶

Summary

• Extracts user-event incorporation into a reusable helper.
• Re-reads and incorporates events immediately before model execution.
• Adds deterministic cadence and reset regression coverage.

Diagram

sequenceDiagram
  participant UI as UI thread
  participant Buffer as EventBuffer
  participant Model as Model loop
  Model->>Buffer: Read current events
  Model->>Model: Prepare run and pace
  UI->>Buffer: Append event during pacing
  Model->>Buffer: Read fresh events
  Model->>Model: Incorporate events
  alt Step remains runnable
    Model->>Model: Execute upcoming step
  else Terminal lifecycle event
    Model->>Model: End loop
  end

Loading

_{Reviews (7) · Last reviewed commit: "Fix reset timing at the paced input snap..."}

[gtong-nv]

The model loop snapshots EventBuffer before _pace(). Input arriving during that cadence wait reaches the following model step, adding up to one configured step interval.

Thanks @ZenAlexa , this is indeed an issue. With our current design, we need to do two paces for the model loop and UI loop.

[[collect events] -> [run] -> [model pace for BACKPRESSUE]  -> [present] -> [UI/present pace for PRESENTATION MODE]] -> [[collect events]

Your PR should address issue with the model pace order

read events A
    |
    +------ model rate-limit wait ------+
                                        |
                 event B arrives here --+--> not included
                                        |
                              model.step(N, A)

But model pace will only happen when BLOCK mode is used in BACKPRESSUE, and happens when model generation is faster than UI rate (which defaults to 60hz), which is rare.
I would suggest clear the PR to only address this issue.
The other changes listed below, don't seem to be relevant

make finished-loop settlement atomic with both reset generation and asynchronous message acceptance
execute every message accepted before the terminal cutoff as one finite batch, then resume generation when that batch reopens model state

[ZenAlexa]

Yep, I agree on keeping this PR tight 👍

One detail I found while tracing it: _pace() is currently driven by frames_per_second_for_step on every model-loop iteration. BackpressureMode.BLOCK adds a separate wait when the presentation queue fills. The ordering issue therefore exists across both modes, and its visible cost is largest when pacing actually sleeps.

I’ve trimmed #573 down to the ordering change and one focused regression. The terminal message settlement is a real lifecycle issue with its own boundary, so I’ve taken it out of this PR and will keep that work separate.

[ZenAlexa]

Caught up with #548's multi-session lifecycle in 8e174e70.

The model loop now reads once before cadence for lifecycle and finished-state decisions, then folds in one more event-buffer snapshot after cadence before the step. Queued state messages keep their once-per-model-step ordering. The focused CPU suite passes all 61 tests (•̀ᴗ•́)و

[ZenAlexa]

Yep, the race exists on current main. #573 intentionally removed that lifecycle change in f7913e96, and the current two-file diff leaves terminal settlement unchanged, matching the scope request above. invoke_async currently documents queued shutdown drops. This makes it a lifecycle-contract decision, and the cadence ordering diff remains isolated. I split that decision into #578.

[gtong-nv]

event_buffer.read(reader_id) is called twice in this loop.
here and in line 333.
This doesn't seem right.. I will look into this

[ZenAlexa]

The second event_buffer.read(reader_id) is intentional and defines the cadence boundary:

• The first read feeds _begin_run() before the cadence wait. It handles lifecycle events, reset generation, and the pre-wait snapshot.
• The second read runs after the wait and advances the same reader cursor. It collects events appended during the wait and folds them into the pending events through _incorporate_user_events() before step().

EventBuffer.read() advances each reader to the buffer end, so the first snapshot is not duplicated. The regression test_model_loop_collects_input_after_pacing appends a keyboard event from the patched cadence wait and verifies that it reaches the next model step. The focused test_session_runner.py suite passes 61 tests on this head.

[greptile-apps]

Post-cadence messages are dropped

When an asynchronous model-loop operation is accepted during cadence pacing and the second event read observes a close event or another terminal condition, _incorporate_user_events() terminates the loop without running another message batch. _shutdown() then discards the accepted operation, causing reset, restart, or other model-state requests to be silently lost.

[ZenAlexa]

I've synced the cadence fix with current main and rerun the input-buffer and session lifecycle tests.

The regression continues to reproduce the stale input on the base branch and passes with the post-wait read.

[ZenAlexa]

I checked the post-cadence queue concern against the shutdown contract: queued operations are discarded when the loop closes.

Reset events use the event-buffer generation, and session replacement goes through the UI loop. I'm keeping one message snapshot per iteration here.