Skip to content

fix: local/instant slash-command turns no longer wait on a Stop sentinel that never fires#16

Merged
htlin222 merged 1 commit into
mainfrom
fix/local-command-stop-sentinel
Jul 5, 2026
Merged

fix: local/instant slash-command turns no longer wait on a Stop sentinel that never fires#16
htlin222 merged 1 commit into
mainfrom
fix/local-command-stop-sentinel

Conversation

@htlin222

@htlin222 htlin222 commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • A turn whose prompt is a slash command handled ENTIRELY client-side (/rename, /branch, /context, /usage, /clear, /model <name>, /effort <level>, /fast on|off, /tui fullscreen) never runs a model/agent turn, so the Stop hook — and its VHS_TURN_DONE_N sentinel — never fires. gen_capture_tape.py's render() still emitted Wait+Screen@<turn_to>s /VHS_TURN_DONE_N/ for these turns, reliably burning the full turn timeout instead of the documented workaround (rephrasing as natural language, sacrificing the real menu).
  • render() now detects these instant commands by leading token (handling args) and emits a short fixed Sleep 1.8s (INSTANT_SETTLE) instead of the sentinel wait — enough for the client-side UI to redraw before the tape moves on. The turn's capture.json plan entry is marked "instant": true (and drops the never-true sentinel field) so downstream stages know not to expect a Stop-hook JSONL entry.
  • Investigated detect_anchors.py/splice.py/author.py/ledger.py: none of them consume session-timeline.jsonl or the sentinel field — turn timing there is derived purely from the captured video signal, so they're unaffected.
  • panel.py DOES read session-timeline.jsonl and indexed ups[ti]/stop[ti] by raw turn number. Confirmed UserPromptSubmit still fires for a client-side command (so ups stays 1:1 with every turn) but Stop never does (so stop only has one entry per REAL turn) — the old code would silently pair every turn AFTER an instant one with the WRONG Stop event, corrupting tool-event placement and conclusion text for the rest of the session. Added _turn_stop_events() to zip ups by turn index but stop by position-among-real-turns, so instant turns are skipped cleanly instead of shifting everything after them.

Part of #1 (bug 4/5: local-command Stop sentinel). See #1.

Test plan

  • pytest tests/ baseline: 98 passed before this change
  • Added regression tests (written before the fix, per this repo's testing ethos) in tests/test_capture_tape.py and tests/test_panel.py; confirmed they FAIL before the fix and PASS after
  • pytest tests/ after fix + rebase onto latest main: 125 passed, 0 failed

…only command

A turn whose prompt is handled ENTIRELY client-side by Claude Code (/rename,
/branch, /context, /usage, /clear, /model, /effort, /fast on|off, /tui
fullscreen) never runs a model/agent turn, so the Stop hook — and its
VHS_TURN_DONE_N sentinel — never fires. gen_capture_tape.py's render() still
emitted Wait+Screen@turn_to/VHS_TURN_DONE_N/ for these turns, reliably burning
the full turn timeout instead of the workaround (rephrasing as natural
language, which sacrifices the real menu).

render() now detects these instant commands by leading token (handling args)
and emits a short fixed Sleep (1.8s, INSTANT_SETTLE) instead of the sentinel
wait, and marks the turn's capture.json plan entry `instant: True` (dropping
the never-true `sentinel` field) so downstream stages know not to expect a
Stop-hook JSONL entry.

Investigated detect_anchors.py/splice.py/author.py/ledger.py: none of them
consume the timeline JSONL or the `sentinel` field — turn timing there is
derived purely from the captured video signal, so they're unaffected.

panel.py DOES read session-timeline.jsonl and indexed ups[ti]/stop[ti] by raw
turn number. Confirmed UserPromptSubmit still fires for a client-side command
(so `ups` stays 1:1 with every turn) but Stop never does (so `stop` only has
one entry per REAL turn) — the old code would silently pair every turn AFTER
an instant one with the WRONG Stop event, corrupting tool-event placement and
conclusion text for the rest of the session. Added `_turn_stop_events()` to
zip ups by turn index but stop by position-among-real-turns, so instant turns
are skipped cleanly instead of shifting everything after them.

Regression tests added per this repo's ethos (every new failure mode becomes
a test, written before the fix): gen_capture_tape.py's instant-command
detection/Sleep-vs-Wait behavior, and panel.py's turn/event alignment across
an instant turn.

Part of #1 (bug 4/5: local-command Stop
sentinel).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012EwptawkV4dcGhoGefzxgV
@htlin222
htlin222 merged commit b0675c4 into main Jul 5, 2026
5 checks passed
@htlin222
htlin222 deleted the fix/local-command-stop-sentinel branch July 5, 2026 04:59
htlin222 added a commit that referenced this pull request Jul 5, 2026
…it (#19)

#1's client-side-command fixes ended up covering two non-overlapping lists:
#16 (INSTANT_COMMANDS) skips the Stop-sentinel wait for no-menu commands
(/rename, /model, ...), while #14 (qmonitor) drives the on-screen menu for
/theme, /rewind, /memory, /plugin install — but nothing told the CAPTURE
tape that THOSE turns also never fire a Stop hook. A live tmux probe
confirmed selecting a /theme option returns to an empty prompt with zero
Stop hook event, exactly like the #16 commands, so gen_capture_tape.py was
still emitting Wait+Screen@120s /VHS_TURN_DONE_N/ for them — a sentinel
that will never print, timing out the full turn-timeout even though
qmonitor was already correctly answering the menu underneath.

Adds NATIVE_MENU_COMMANDS (/theme, /rewind, /memory, /plugin install only
— other /plugin subcommands don't show the scope-picker menu) matched via
a _match_command_table helper shared with INSTANT_COMMANDS, and skips the
sentinel Wait for these turns too, marking turn_plan["instant"] = True
(the exact same flag/shape #16 already uses, so panel.py's existing
`not t.get("instant")` filter and any other downstream consumer keep
working unchanged).

The settle Sleep is mode-dependent: under --robust/--tmux a real
qmonitor.py is driving the menu (poll -> dwell 2.5s -> keystrokes at
0.8s/key -> confirm poll), so the tape sleeps NATIVE_MENU_SETTLE (10s —
sized from qmonitor's own defaults with headroom, well short of its
60-90s worst-case ceilings) before moving on. In STANDARD (non-tmux) mode
there is no qmonitor at all, so a /theme menu never gets navigated
regardless of sleep length — that usage isn't supported by this fix — but
we still must not hang on an unreachable sentinel, so it falls back to
the short INSTANT_SETTLE there instead.

Closes #17.


Claude-Session: https://claude.ai/code/session_012EwptawkV4dcGhoGefzxgV

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
htlin222 added a commit that referenced this pull request Jul 5, 2026
… scripted timing (#20)

* fix: detect_anchors recovers merged consecutive-instant-turn submissions using known scripted timing

Two client-side-only turns back-to-back (e.g. "/context" then "/fast on") have
no real thinking-gap between them, so the input-band pixel signal never dips
long enough to split them -- they read as ONE continuous group instead of two,
undercounting detected prompt submissions and raising in author.py (#18).

The fix adds a targeted recovery for the len(groups) < n case: when the
turns.json plan already flags a run of >=2 consecutive turns as "instant"
(written by gen_capture_tape.py's render(), consumed via plan["turns"]) and the
shortfall is EXACTLY explained by that run collapsing into one detected group,
split the merged group back apart using the deterministic scripted timing
(INSTANT_SETTLE + pre_enter + each turn's type_dur) instead of re-detecting
split points from pixels -- there is no pixel signal left to re-detect from.
If the shortfall isn't fully explained this way, it still raises: this is a
recovery for a known, deterministic failure mode, not a blanket excuse to
swallow a genuine detection miss.

Regression tests added in tests/test_detect_anchors.py reproduce the exact
3-vs-4 mismatch from the issue with a synthetic merged input-band signal, and
confirm the existing "shortfall not explained" case still raises.

Closes #18.

* fix: consecutive-instant recovery uses each turn's actual settle duration, not a hardcoded constant

The just-landed #17/#19 fix (native CLI menu turns, e.g. "/theme") gives
those turns the SAME turn_plan["instant"] = True flag/shape as the no-menu
INSTANT_COMMANDS from #16, but they can use a LONGER settle
(NATIVE_MENU_SETTLE under --tmux) than the no-menu INSTANT_SETTLE. My
detect_anchors.py recovery (previous commit) assumed every "instant"-flagged
turn used the module's INSTANT_SETTLE constant -- correct for #18's own
"/context" + "/fast on" repro, but silently wrong (not raising, just
mis-timed) for a merged run that includes a native-menu turn.

gen_capture_tape.py now records the settle it actually used per-turn in
turn_plan["settle"]; detect_anchors.py's split reads that value
(turns[idx].get("settle", INSTANT_SETTLE)) instead of assuming one constant,
falling back to the old default for capture.json plans predating this field.

Added a regression test with a mixed run (a native-menu-settle turn followed
by a plain-instant turn) proving the recovery uses the per-turn value.
htlin222 added a commit that referenced this pull request Jul 5, 2026
#23 postmortem) (#24)

PR #23 fixed a real bug where a long wrapping prompt fooled the pixel-based
turn-boundary detector into producing a turn whose "hard" segment was
mathematically impossible (shorter than the deterministic scripted typing
time alone) — silently corrupting the ledger. It was only caught by a human
eyeballing video frames, not by the pipeline itself. This adds two
independent, complementary guards so the SAME class of bug (and others
shaped like it) gets caught automatically next time, instead of waiting for
another reactive patch.

Layer 1 (detect_anchors.py, detect_turns()): a deterministic self-consistency
floor requiring no external data. submit - typing_start must equal exactly
type_dur + pre_enter (the scripted time to type the prompt and reach Enter) —
a real submit can never be faster than that. The existing `max(0.0, ...)`
clamp used to silently absorb a violation of this; now it raises immediately,
naming the turn and the impossible numbers.

Layer 2 (author.py, main()): a best-effort cross-check of the pixel-detected
per-turn response window (done - submit) against session-timeline.jsonl's
real UserPromptSubmit->Stop wall-clock delta for the same turn, when that
file is available and its turns can be unambiguously correlated (mirrors
panel.py's existing instant-turn-aware matching). This catches a DIFFERENT
class of error Layer 1 cannot: a wrong-but-internally-self-consistent group
selection that still satisfies Layer 1's floor. It degrades silently on a
missing/malformed/uncorrelated timeline — never a hard requirement — and only
raises on a wild disagreement (tolerance: max(5.0s, 50% of the real delta),
chosen generously against detect_turns()'s own documented "many seconds"
video/wall-clock drift risk during heavy output, while still comfortably
catching a mismatch of the #23 bug's scale; validated against one real
110s-recording data point showing ~7% pixel-vs-wallclock agreement).

Layer 2 intentionally lives in author.py, not detect_anchors.py: detect_turns
already argues wall-clock timing can't be the PRIMARY source of truth (video
drift), and author.py is the orchestration layer that already knows the demo
path, per CONTRIBUTING.md's pure-logic/thin-I/O split.

12 new regression tests (3 Layer 1, 9 Layer 2); full suite green (151 passed,
up from the pre-existing 139), including every #16-#23 recovery-path
regression test unmodified.


Claude-Session: https://claude.ai/code/session_012EwptawkV4dcGhoGefzxgV

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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.

1 participant