Skip to content

fix(settle): a blank capture never stands in for the tree check - #139

Merged
leeguooooo merged 1 commit into
mainfrom
perf/settle-frames-first
Oct 6, 2026
Merged

leeguooooo merged 1 commit into
mainfrom
perf/settle-frames-first

Conversation

@leeguooooo

Copy link
Copy Markdown
Owner

Two fixes. One finding that did not pan out is recorded here so nobody repeats it.

  1. Blank frames. An app that hides its screen from capture (PayPay) gives the same blank frame whether or not it moved. Two identical blank frames around the tree read therefore declared a moving screen settled. Blank frames (flat content band, redaction::content_band_is_blank) now fall back to comparing trees. Test: blank_frames_never_count_as_settled.
  2. Test fixture. FRAME_B was non-canonical base64 and failed to decode, so frames_that_differ_fall_back_to_comparing_trees was really exercising a failed capture.

Tried, not kept: wait for frames to hold still before the first tree read.
Hardware: v0.13.1, Settings 通用 → 关于本机 → back → back, observe on, USB.

  • Baseline: mean 6.1 s per step.
  • 2 s cap: 4.7 s on the first run, 5.9 s on the second.
  • 4 s cap: 6.0 s.

Per page:

  • Plain transitions (通用, some backs) dropped from 6.5 s to about 3 s, with one /source read.
  • Pages that load content after the transition (关于本机) went from 4.6–6.8 s to about 7 s. The frames held still, the content changed during the 1.4 s tree read, and a second read followed anyway.

The net change is within noise, so the behaviour is not merged. The bigger lever is the cost of each tree read itself: the wechat-project session found that isVisible dominates /source time (heavy-tree lite-source work).

server + mcp: 598 passed.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 22 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b075309f-17d4-47a4-971b-43e123af2367
📥 Commits

Reviewing files that changed from the base of the PR and between 2ee575a and 421b6d7.

📒 Files selected for processing (2)
  • crates/server/src/http.rs
  • crates/server/tests/agent_input_settle.rs
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…ir the frame fixture

- An app that hides its screen from capture (PayPay) yields the same blank
  frame whether or not it moved, so two identical frames around the read
  declared a moving screen settled. Blank frames now fall back to comparing
  trees.
- FRAME_B in the settle tests was non-canonical base64: it failed to decode,
  so 'frames that differ' was really testing 'the second capture failed'.

Also tried (not kept): waiting for frames to hold still before the first tree
read. On Settings it sped up some steps (6.5 s -> 3 s) and slowed pages that
load content after the transition (about 关于本机: 4.6-6.8 s -> 7 s); the mean
moved 6.1 s -> 5.9 s, within noise.
@leeguooooo
leeguooooo force-pushed the perf/settle-frames-first branch from 34373f0 to 421b6d7 Compare October 6, 2026 10:53
@leeguooooo
leeguooooo merged commit fa4d95c into main Oct 6, 2026
2 checks passed
@leeguooooo
leeguooooo deleted the perf/settle-frames-first branch October 6, 2026 11:00
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