Repository navigation
Hide Timeline indicators and expand Detail on visible demand - #63
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a88e325a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if List.mem id seen | ||
| then [] | ||
| else ( | ||
| let loading = read_pending t in |
There was a problem hiding this comment.
When an outline contains multiple unresolved branches, read_pending t makes every More row report loading = true while the single serialized request is active. detail_page consequently replaces unrelated Load more/Retry controls and their error feedback with “Loading children” spinners, even though those branches have no request in flight; this can persist through the full automatic-read sequence. Derive loading from the current branch's pending field, and disable other actions separately if serialization is required.
Useful? React with 👍 / 👎.
|
|
||
| (* Root is level one; typography independently caps at three levels. *) | ||
| let maximum_depth = 9 | ||
| let maximum_automatic_reads = 128 |
There was a problem hiding this comment.
On a wide, nested outline, this permits 128 background reads of up to 128 children each, so merely opening Detail can materialize over 16,000 block rows without user interaction; unresolved descendants can add similarly many More rows. Every completion then rebuilds the flat OCaml row list and its labels before handing it to the native list, causing substantial read amplification and repeated main-thread work. Apply the automatic budget to returned blocks/visible rows rather than request count so the default expansion cannot grow this large.
Useful? React with 👍 / 👎.
RCmerci
changed the title
1 participant
Timeline hides its native vertical scroll indicator while Favorites and Detail retain theirs. Detail presents root depth 0 through 9 without disclosure controls, retaining three typography tiers and full-subtree Copy. Detail now uses 24/19/15pt, down 2pt per tier from 26/21/17; Dynamic Type, tier clamping, target line heights and paragraph gaps remain unchanged.
Automatic child discovery now follows the scoped native visible range. Opening a wide outline no longer probes every unknown offscreen branch. Returned pages and empty-page attempts have separate bounds (128 items/page, 384 conservative block units per automatic round, at most 128 attempts); continuation and failed pages remain explicit Load more/Retry actions. A pending branch alone shows loading, while unrelated actions disable without hiding their error feedback and explain the wait even when the pending branch is offscreen. Same-outline row snapshots are reused across draft, range, and no-op updates.
Public production projection regressions preserve unknown-zero versus authoritative empty children, ancestor-path cycle handling, pagination/order, stale-session fencing and media ownership. Independent before/after metrics: the wide fixture falls from 128 reads / 16,384 returned children / 17,409 rows to 1 read / 128 children / 512 rows. A narrow native index window probes 2 unknown leaves rather than all 128; deep ten-level data still requires 8 reads. Loaded-node duplicates remain zero. Changing structure still rebuilds one snapshot; label/media mapping and user-driven cumulative pages remain existing work. This is bounded visible-demand expansion, not a universal zero-work claim.
Validation: 40 route, 44 semantics, 98 runtime, and 76 Application cases passed; build @all/native_embed and the complete Journal regression suite passed. Independent implementation-time review found no confirmed unresolved findings. Fixed-head Simulator acceptance: seven official XCTest scenarios passed for branch feedback/Retry, wide visible-demand work, append/Back/target drafts, Timeline-only indicators, ten levels/full Copy and 128/128/4 paging. The final wide UI session contains 65 wire requests, including 2 children requests (root+visible branch); metadata/media requests are reported separately. Full 260-child viewport traversal necessarily discovers 260 unknown empty leaves in addition to the 3 root pages under the present contract; no universal zero-read claim. Exact head 41b591e passed Journal CI (run 38130036587). The persistent independent session reviewed cc1d931 (12 public-interface experiments and 257 candidate cases); its same-session incremental review of the final two-file caption/test change completed with no new P0/P1/P2 or blocking findings. It independently recompiled the new focused Application test, reread the 3+4 xcresult passes and verified the actual host/installed binary hashes. The offscreen wait-reason P3 is fixed. The existing cumulative List scale P3 remains a stated limitation.
The four baseline Capture UX findings are tracked for a separate owner/PR and are not changed here. No spec, Dune, production account/graph or iPhone changes.
Typography follow-up: exact head f50436f changes one Swift line. Correct native-size RED failed on 41b591e; final real Simulator normal and SwiftUI accessibility3 multi-level tests passed (2/2), including 24/19/15 intrinsic line-height checks, deep third-tier reuse, wrapping, scrolling and Back/Capture reachability. Built and installed native binaries match SHA256 95509801e3a0a3298029d5bfae980c1d50b68ff49f1976dabae49238dcf13128. Reused Application/C object, fixed LUI/Signal and Swift caches; no local full Journal/LUI rebuild. 44 public semantics regressions passed. Exact new-head Journal CI passed (run 38133943281: all targets/native_embed and full regression suite). Same-session incremental independent review completed with no new confirmed issues or blockers, independently checking fixed Git inputs, built/installed binary and raw 2/2 xcresult evidence. The existing cumulative List P3 remains unchanged; earlier 41b591e evidence remains historical.