Skip to content

fix: retain visible result windows during refresh - #123

Merged
tiensonqin merged 7 commits into
mainfrom
devin/1793521237-table-stationary-blank
Nov 1, 2026
Merged

tiensonqin merged 7 commits into
mainfrom
devin/1793521237-table-stationary-blank

Conversation

@tiensonqin

@tiensonqin tiensonqin commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Keep the presented deferred-result window mounted while the same query is revalidated, instead of replacing a far-row window with the snapshot's initial 12 rows before its replacement arrives.

  • Separate cache freshness from presentation coverage. Retained rows cannot satisfy a new generation's cache, but still protect visible coverage: a partially overlapping fresh reply cannot displace a more complete visible page. Clamp that comparison to the replacement's row count so shrinking/empty results can remove obsolete rows.
  • Keep the requested range covering the current viewport, including when it moves back inside a retained page while another read is outstanding. Completed replies still require matching input, transaction revision, and projection revision.
  • Add the session-local projectionRevision to snapshot views and row-window replies. Physical database replacement or a local journal-day change invalidates it even when the transaction number and initial rows are identical. Query snapshots share the session token without reusing context-sensitive supplied rows from the prepared-view cache.
  • Reset range and viewport together for input/settings/type changes. Graph changes, errors, results shrinking past the viewport, and legitimate empty searches do not retain unrelated rows. Initial reads remain 12 rows; prefetch remains bounded to 64.
  • Strengthen the existing SwiftUI/Kotlin pagination contract for far-row-only equal-transaction refresh and refresh/scroll/partial-reply interleaving. Add default-off LOGSEQ_TRACE_RESULT_WINDOWS=1 metadata diagnostics; these distinguish source invalidation, actual requested ranges, and shared page acceptance, not native render completion.
  • Fix the existing sidebar flow's obsolete dismiss selector without weakening assertions or either edge swipe, and pin LUI's compact closed-panel geometry fix from logseq/lui#301. The retained panel parks fully offscreen only at rest; regular split layout, identity, hit-testing and accessibility policy are unchanged. Maestro's snapshot presence is not VoiceOver focusability.

Validation at 629f1963: the far-row-only freshness assertion fails before its fix, and the partial-fresh-window assertion fails on 3c242752. Both now pass in the existing contract. All 1,257 core tests pass locally with opam exec --switch=5.5.0 -- dune build @shared/native/runtest; the focused paging/result-view run passed all 46 tests. git diff --check passes.

Dependency follow-up at 01f1beca: OCaml and SwiftPM pins agree on published LUI ca87d1cdf534954a38f55616320ede8b7ac6d91e (compatible drawer line, not LUI main). SwiftPM regenerated Package.resolved, including its origin hash; the Linux and Mac three-file diffs are byte-identical. Local Android unit tests (82), lint, debug and canonical native/profile builds pass, as do JNI-boundary and packaging checks. The canonical iOS Release simulator app build passed in 128.02s using the public checkout, without path overrides; no UI run of this newly built app is claimed. The earlier byte-identical LUI production candidate passed the unchanged compact sidebar flow, actual controls/Capture, regular iPad basics and real 37pt Recent-section scroll retention (two UUID bounds unchanged after reopen). Current client non-root sidebar routing is N/A; cancellation, VoiceOver, theme, physical devices and Table/passive acceptance remain untested. LUI's follow-up test-only commit migrates two existing tracker tests to required registration UUIDs without changing their assertions or production code. Latest CI: 4 passed, 1 failed. iOS e2e stopped during Dune host ppxlib AST generation before Maestro: the installed ppxlib.ast/ast.ml input was missing after a cross-dependency build-cache restore. The cache paths omit _build/install, and a failed build was saved as a partial cache. A controlled A/B with and without _build/install did not rerun the generator during the canonical build, so the complete CI root cause remains UNKNOWN; no workflow fix has been committed. Earlier UI CI results below describe the previous revision.

Formal-app verification of 629f1963 is not an overall pass:

  • Android profile emulator: original 302×9 Graph_store fixture, Name/Tags left edge, 0300–0289 starting viewport, no overlay. The complete original three 180 ms flings ran from the correct starting state without observed full-body blank or local gap. Separate vertical-boundary and horizontally-offset vertical checks still showed approximately 33–34 ms local bottom-row gaps; cold-open fill was approximately 67 ms, and empty-search return fill approximately 467 ms. Far-row Name edit/new-value/restore and empty-search replacement worked. Passive/equal-revision refresh was not tested; mirroring/VFR cannot establish native horizontal stall duration.
  • iOS Release simulator: final source/dependency/core/link/installed identity and the immutable 1,000×11 fixture were verified. After R1, independent setup aligned the historical Row963-partially-obscured/Row962-first-complete, Name+Field1 idle anchor ([0,0] translation, not pixel identity). The single idle interval was 38.437 s (38.586 s including boundary screenshots). Sync/snapshots occurred, but diagnostics showed no target source/generation advance, so passive/equal-revision refresh remains untested, not passed. The gate stopped further 3V/3H, R2 and mutation checks. No confirmed blank/local gap was found in the inspected samples; this does not prove the untriggered path is fixed. Normal Capture setup changed the app working copy before the interval; protected immutable fixture hashes remained unchanged. No physical-device or exact native mount/stop timing evidence is available.

On the earlier revision, Android CI smoke passed; iOS e2e failed at the closed-sidebar visibility assertion above. Neither CI nor shared accept traces establish native Table render completion. A separately labeled Android diagnostic package reproduced gaps after the missing rows were already in the shared accepted page; the exact native stage is still unknown. A subsequent viewport-sized mounted-overscan experiment was rejected and removed: its sampled scroll paths had no detected gap, but search-return bottom-row refill persisted for up to 45 raw frames (~750 ms), and cold-open refill took ~117 ms. These are diagnostic/candidate results, not formal acceptance or paired A/B timings. The formal APK/core were restored. The first native-diagnostics APK incorrectly omitted non-signature META-INF entries and is not valid isolation evidence. A corrected package retained all 93 non-signature META-INF entries, with verified payload/signature/core/fixture identity, and repeated the same bounded scenario. Its scrolling gap occurred after shared accepted rows covered the viewport: native visible range was 57–70 while the mounted slice still ended at 66. Range-to-publication was about 50 ms, publication itself below 1 ms, then about 68 ms to Compose and 17 ms to a complete draw-command pass. Search-return's initial 12-row gap still lacks proof that fresh prefetched data had arrived. Draw commands and mirrored frames do not establish physical display presentation.

One subsequent 100 Hz Perfetto run on the unchanged formal APK again reproduced a local row gap (~83 ms in mirrored raw frames). Actual Main stacks included runCore → flushUiCalls → visibleRange → applyBatch → publish/refresh, plus native effect/snapshot and Compose work; worker stacks reached OCaml Result_view.row_json and graph/index reads. Sampling does not provide phase boundaries, queue residence time, or a unique root-cause attribution. The third boundary was delayed by a setup check; the gap was at the preceding second boundary. All temporary Kotlin diagnostics were removed and the nested LUI checkout is clean; app/capture/profiling processes were stopped and formal APK/core plus fixture identity reverified. The remaining native rendering issue is unresolved. A subsequent read-once cell-property candidate passed 46 focused/1257 core tests but failed the same Android UI scenario: a third-boundary local row gap persisted for ~83 ms in mirrored frames. That candidate was reverted in full; the core/prefetch/mounted-window code is unchanged. A tail-only deferred-range coalescing candidate then passed 83 Android unit tests/lint, but also failed: a second-boundary local row gap lasted 7 mirrored raw frames (~117 ms), with no whole-body blank detected in 5211 sampled frames. Its canonical APK differed only in app DEX; all native libraries and non-signature metadata matched formal. This candidate was also reverted, and formal APK/core/fixture state restored. The Android/LUI working trees are clean; 82 baseline Android unit tests and lint pass. Neither run measures physical presentation, actual coalescing counts, unique root cause, or a paired A/B performance improvement.

Follow-up to #122. The historical 100–133 ms iOS idle blank is confirmed; green core/CI checks alone do not prove that it or every gap/stall has been eliminated.

Link to Devin session: https://app.devin.ai/sessions/f2fb979c38d945a9a836b086a5132f95
Open in Devin Desktop: https://app.devin.ai/desktop/session/f2fb979c38d945a9a836b086a5132f95?variant=devin
Requested by: @tiensonqin


Devin Review

@devin-ai-integration

Copy link
Copy Markdown

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin resolved the finding on 629f196

Fixed by Devin (1)

  • Empty searches retain unrelated rows

View all findings in Devin Review

Devin Review

@tiensonqin
tiensonqin merged commit e260b34 into main Nov 1, 2026
4 of 5 checks passed
@tiensonqin
tiensonqin deleted the devin/1793521237-table-stationary-blank branch November 3, 2026 08:25
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