Repository navigation
Fix forced body redisplay and add account-free Telega GUI coverage - #472
Merged
Merged
Conversation
A display image spec can change in place without moving buffer modification ticks. image-flush on the newly mutated spec can also leave the old source's catalog generation unchanged. force-window-update previously moved only the generic redisplay generation, which retained body rows did not consult, so incremental layout reused stale geometry after an explicit forced update. Model all-window, window and buffer requests with ForcedBodyRedisplay and carry an opaque BodyRedisplayRevision through the retained layout key, snapshot freshness and speculative layout freshness. Keep presentation-only redisplay separate so mode-line/menu updates preserve reusable body rows. Follow GNU Emacs Fforce_window_update and REDISPLAY_BUFFER_WINDOWS: refresh mode lines on successful requests; accept displayed buffers and names; search exact buffer identities on visible frames, excluding even active minibuffers; return nil for invalid, dead, hidden or undisplayed targets. Reset the new runtime revisions on context construction and pdump reconstruction. Preserve explicit forcing of a live minibuffer window, with a regression that failed before correcting the buffer-target walk. The mutable-image geometry regression failed before the fix. Add scope, mode-line and indirect/hidden-buffer regressions, a live GNU return-value oracle and a paired terminal display-spec mutation test. Verified 117 layout and 15 core tests, two live GNU oracle tests and two terminal tests after rebasing onto main, including its new terminal-output regression. The oracle and forced-repaint terminal checks passed three repetitions without retries using the freshly rebuilt development binary. This is a separately reproduced editor bug discovered while investigating missing Telega avatars; it does not establish the cause of that original personal-session report.
Replace Telega's configurable telega-server subprocess with an offline Rust fixture while keeping the pinned Lisp frontend, process filter, callback dispatch, SVG construction, redisplay and paging real. Model framed UTF-8 plist requests and typed events with callback correlation; reject unsupported requests and unknown scenario identifiers instead of silently succeeding. Generate 112 synthetic group chats and PNG avatars, with controlled delayed file delivery and photo replacement. Provision Telega and its locked dependency closure through neomacs-infra rather than loading personal packages, Telegram data, credentials or captured conversations. Give each scenario a fresh HOME/XDG/temp/runtime tree, authenticated private Xvfb display and owned editor/mock process group. Strip user display, D-Bus and loading overrides. Verify avatar glyphs and screenshot pixels within each visible row, real PageDown/PageUp motion, delayed repaint, replacement and cleanup after both normal exit and panic. Cleanup signals only the recorded group, never processes by name. Verified the package pin, 23 fixture unit tests and all seven GUI tests. The unit and private GUI suites each passed three repetitions with retries disabled; final GUI checks used the freshly rebuilt development binary. Document provisioning, commands, artifacts and the configuration-isolation boundary: no OS network/filesystem sandbox or TDLib/MTProto coverage. The fixture also passes before the separate retained-layout fix, so these tests provide reproducible frontend coverage without claiming reproduction or resolution of the original missing-avatar report.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (26)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The change touches correctness- and performance-critical redisplay/layout-reuse invalidation and adds a very large new GUI/subprocess test surface, which warrants final human review despite no concrete defects being found.
Review effort: Balanced
Findings: None
What changed in this PR
This PR fixes a correctness gap in force-window-update and adds account-free integration coverage for the Telega frontend. Previously, force-window-update advanced only the generic redisplay_generation, which also moves for presentation-only work (chrome/menu/mode-line). Because an image/display spec can be mutated in place without changing buffer ticks or an image catalog generation, retained body rows could keep stale geometry after an explicit force. The fix introduces a dedicated, typed, body-only invalidation channel that participates in retained-layout keys and freshness checks, so an explicit force refuses body-row and scroll reuse without escalating unrelated windows. The second part adds a real-frontend, subprocess-mock Telega GUI test that drives paging and checks per-row avatar glyphs/pixels under fully isolated sessions.
Changes:
- Add
ForcedBodyRedisplay(typed scope) andBodyRedisplayRevision(per-all/window/buffer revision) and thread them throughContext, retained window keys, and synchronous/speculative freshness checks so forced body redisplay cannot be aligned away by scroll-surface compatibility. - Rewrite
builtin_force_window_updateto follow GNU return values and visible-frame/exact-buffer matching, exclude active minibuffers for buffer-target forcing while still accepting explicitly forced live minibuffer windows, and raise the global mode-line trigger independently of body invalidation. - Add account-free Telega GUI coverage (protocol mock, synthetic scenario, mock
telega-serverbinary, package-lock pin) plus oracle/unit/TUI regression tests.
| File | Description |
|---|---|
crates/neovm-core/src/window/mod.rs |
Adds ForcedBodyRedisplay enum, BodyRedisplayRevision struct, and window_buffer_id helper; wires body-redisplay term into freshness structs. |
crates/neovm-core/src/emacs_core/runtime/eval/mod.rs |
Adds per-all/window/buffer body-redisplay counters to Context with documentation separating them from redisplay_generation. |
crates/neovm-core/src/emacs_core/runtime/eval/command_loop.rs |
Implements force_body_redisplay, mark_buffer_body_redisplay, and body_redisplay_revision. |
crates/neovm-core/src/emacs_core/runtime/eval/construct.rs |
Initializes/resets the three new counters at the two construction/reset sites. |
crates/neovm-core/src/emacs_core/runtime/eval/pdump_reconstruct.rs |
Initializes the new counters during pdump reconstruction. |
crates/neovm-core/src/window/display.rs |
Captures body_redisplay revision in both freshness-snapshot builders. |
crates/neovm-core/src/emacs_core/display/window_cmds/mod.rs |
Rewrites force-window-update for GNU-parity return values, buffer/visible-frame matching, minibuffer exclusion, and typed body invalidation plus mode-line update. |
crates/neovm-core/src/emacs_core/display/dispnew/tests/mod.rs |
Adds body-redisplay scope regression tests (all/window/buffer, minibuffer). |
crates/neovm-oracle-tests/src/divergence/combos/complex/case_409.rs |
Adds an oracle parity test asserting the GNU object contract (t t t t nil nil nil). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
An image/display spec can be mutated in place without changing buffer ticks or the old image source's catalog generation.
force-window-updatepreviously advanced only the generic redisplay generation, so retained body rows could keep stale geometry after an explicit force.Add scoped
ForcedBodyRedisplayrequests and opaqueBodyRedisplayRevisionvalues to retained layout keys and synchronous/speculative freshness checks. Follow GNU Emacs's return values, visible-frame/exact-buffer matching, and mode-line updates. Buffer-target forcing excludes active minibuffers; explicit live-window forcing still accepts them. Presentation-only updates continue to reuse body rows.The second commit adds account-free integration coverage using the real pinned Telega frontend and a Rust subprocess fixture at
telega-server-command. It generates 112 synthetic chats and PNG avatars, drives actual PageDown/PageUp, checks leading-avatar glyphs and pixels per visible row, and exercises delayed file delivery and photo replacement. Unsupported requests and unknown scenario identifiers fail explicitly.Every GUI scenario owns an authenticated private Xvfb display, fresh HOME/XDG/temp/runtime directories, and an editor/mock process group. Normal and panic cleanup tests require authenticated connectivity before teardown and verify its disappearance afterward with preserved credentials. No personal Telegram account, configuration, data, server, or user display is used.
Evidence and limits
The mutable-image retained-layout regression failed before the production fix. Review also caught an active-minibuffer mismatch; its new regression failed before the correction. Both review blockers are resolved.
The original personal-session missing-avatar report remains unconfirmed. The revised Telega fixture already passed before the production change, so this PR does not establish that report's root cause or claim to fix it. The fixture tests the frontend/process/rendering boundary, not TDLib, authentication, MTProto, or Telegram's service. Isolation is configuration/session isolation rather than an OS sandbox; GUI coverage is Linux/X11 and host dependent. Provisioning may download pinned sources before scenarios start.
Validation
Rebased onto
mainat9ab7e16442, then rebuilttarget/debug/neomacsand passed:send-string-to-terminalregression.Fixture units, GUI, oracle, and forced-repaint PTY tests passed three repetitions without retries. After review, strengthened normal/panic cleanup tests and all 15 core tests also passed three repetitions without retries. Final integration checks used the rebuilt development binary.
Commands, source research, coverage boundaries, and isolation details are documented in
crates/neomacs-gui-tests/fixtures/telega-fixture.md.