fix: detect_anchors only merges consecutive instant turns sharing the same settle duration#22
Merged
Merged
Conversation
… same settle duration #20's recovery treated a whole maximal run of consecutive `instant`-flagged turns as one atomic collapse candidate (either all merge into one detected group, or none do). Real pixel behavior isn't atomic once settle durations differ within a run: a NATIVE_MENU_SETTLE (10s) turn is long enough to stay its own visually-distinct group even sandwiched between short INSTANT_SETTLE (1.8s) turns, so only the short-settle sub-run actually merges (issue #21). Fix: if the whole-run collapse doesn't exactly explain the shortfall, retry using maximal SAME-settle sub-runs within it as the collapse candidates instead (e.g. "/theme" stays isolated, "/context"+"/fast on" merge). The original whole-run attempt runs first and unchanged, so every existing recovery path (including the per-turn-settle test where a run already fully collapses) is untouched; the sub-run fallback only kicks in for shortfalls the whole-run math can't already account for. Closes #21. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012EwptawkV4dcGhoGefzxgV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #21.
Problem
#20's recovery in
detect_anchors.pyfound maximal runs of>=2consecutiveinstant-flagged turns and treated each run as one atomic collapse candidate — either the whole run merges into a single detected pixel group, or it doesn't. That's wrong once settle durations differ within a run: a native-menu turn (/theme,NATIVE_MENU_SETTLE=10s) is long enough to stay its own visually-distinct group even when immediately followed by short-settle instant turns (/context,/fast on, eachINSTANT_SETTLE=1.8s) that DO merge with each other.Confirmed repro from the issue:
[normal, normal, /theme, /context, /fast on]underrecord-session --robust. Capture succeeds;authorfails withdetected 4 prompt submissions, expected 5, because #20's recovery computed "the whole 3-turn run fully collapses" as a shortfall of 2, which doesn't match the actual shortfall of 1 (only/context+/fast onmerged).Fix
_recover_merged_instant_groupsnow tries recovery in two passes:_split_runs_by_settle) and retry the same exact-match recovery per sub-run. A turn whose settle differs from its neighbor's is a real dividing line (e.g./themesandwiched between/context+/fast on), so a lone differently-settled turn is never considered a merge candidate.Exact-equality of the recorded
settlevalue was used as the grouping key rather than a numeric threshold — there are currently only two known settle constants (INSTANT_SETTLE=1.8,NATIVE_MENU_SETTLE=10.0), so equality is a simpler, more conservative proxy for "these turns visually merge" and avoids guessing at an arbitrary threshold between the two.Testing
test_detect_turns_recovers_partial_merge_within_mixed_settle_run: the exact issue 3+ consecutive instant-flagged turns with mixed settle durations still break detect_anchors's recovery #21 repro (5 turns,/themestays distinct,/context+/fast onmerge) — now correctly recovers 5 groups.test_detect_turns_recovers_full_collapse_of_same_settle_run_of_three: a run of 3 same-settle instant turns still fully collapses via the unchanged first-pass logic (no regression).test_detect_turns_raises_when_mixed_settle_shortfall_not_explained: a genuine unexplained shortfall inside a mixed-settle run still raises rather than being silently swallowed.Not done
Did not perform a real end-to-end
record-session --robustrepro (needsvhs/tmux/edge-tts/real Claude credentials and several minutes) — per this repo's own CI and CONTRIBUTING.md, unit tests are the primary bar, and all pure-logic paths are covered by the new regression tests above.Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_012EwptawkV4dcGhoGefzxgV