Repository navigation
fix: retain visible result windows during refresh - #123
Merged
Merged
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
There was a problem hiding this comment.
✅ Devin resolved the finding on 629f196
Fixed by Devin (1)
- Empty searches retain unrelated rows
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.
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.
projectionRevisionto 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.LOGSEQ_TRACE_RESULT_WINDOWS=1metadata diagnostics; these distinguish source invalidation, actual requested ranges, and shared page acceptance, not native render completion.Validation at
629f1963: the far-row-only freshness assertion fails before its fix, and the partial-fresh-window assertion fails on3c242752. Both now pass in the existing contract. All 1,257 core tests pass locally withopam exec --switch=5.5.0 -- dune build @shared/native/runtest; the focused paging/result-view run passed all 46 tests.git diff --checkpasses.Dependency follow-up at
01f1beca: OCaml and SwiftPM pins agree on published LUIca87d1cdf534954a38f55616320ede8b7ac6d91e(compatible drawer line, not LUI main). SwiftPM regeneratedPackage.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 hostppxlibAST generation before Maestro: the installedppxlib.ast/ast.mlinput 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/installdid 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
629f1963is not an overall pass:0300–0289starting 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.[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
accepttraces 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 OCamlResult_view.row_jsonand 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