Skip to content

NEXUS-4082: Stabilize new-message boundaries and read-only focus - #2083

Open
kazemek21 wants to merge 1 commit into
mainfrom
kylez_NEXUS-4082_channel_unread_boundary
Open

kazemek21 wants to merge 1 commit into
mainfrom
kylez_NEXUS-4082_channel_unread_boundary

Conversation

@kazemek21

@kazemek21 kazemek21 commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

TL;DR

Add a visit-stable “New messages” divider and focus read-only channel history on normal navigation so visible messages can become read. The divider waits for verified history and visit reconciliation. Neither change alters unread eligibility or read-state writes. Fixes NEXUS-4082.

Why

Automatic reading advances unread state. A boundary derived from the current unread cursor could move or disappear during a channel visit.

What

Capture one unread message ID for the current viewer, community, channel, and visit. Keep the divider fixed through reads, arrivals, and temporary timeline unmounts; omit it when the entry boundary cannot be proved. For read-only channels without a composer, hand normal navigation focus to message history once it loads. Preserve explicit focus choices and newer focus owners.

How

Reuse the existing message-visit, unread snapshot, attention, and subscription APIs. The existing visit owner captures loaded row IDs, the read-intent revision, and already-known read evidence before the timeline first mounts. Messages received during an initial loading or seek placeholder cannot become the entry boundary. Retain the boundary and baseline under the existing visit token across timeline remounts. If known entry-row read evidence changes through sync while capture is pending, omit the divider rather than move it to a later row. Keep read eligibility and cursor advancement in their current owners. Reuse ChannelsPage’s history container and content-ready callback for the read-only focus handoff. Keep cancelled or consumed focus intent across connection rebinding. Yield to preserved navigation, newer focus, aborted visits, and hidden or inert content; do not add timers or force-read calls.

Risk

This changes channel timeline rendering, visit-local state, and read-only navigation focus. Existing focus, visibility, membership, and settled-reading safeguards still decide when reading occurs. It does not add another read-state engine or alter read operations.

Testing

The initial divider regression failed without the feature and passed with it. The backdated-arrival regression also failed on 8e70de2: Chromium and WebKit both displayed a divider for a message received after entry. With this fix, the regression passed in both engines and preserved that message’s unread state. Mounted React cases cover backdated, same-second, and forward arrivals.

At 42479be, Blox passed 170/170 focused ChannelTimeline tests, touched-file Biome autofix/lint, and the full boundary browser file (4/4: two journeys in each engine, no retries). The tested source hashes match that snapshot. The previous late-loaded-row positive assertion is replaced with a conservative omission case for initially unloaded rows; entry membership cannot be inferred later.

The five new mounted regressions failed on 42479be before the repair: retained nonempty/empty boundaries, pending read-intent and arrival baselines across a temporary unmount, and a synchronized read without a local revision. On the repaired snapshot, Blox passed 289/289 tests across the six focused timeline, visit, and read-state files, touched-file Biome, and the full two-engine boundary browser file (4/4, no retries). Tested source hashes match 7a47c48. The synchronized-read case controls the consumer ordering in mounted React; it is not a live multi-device reproduction. No browser cases were added or removed for this repair.

Fixture-only cleanup at 7abf46d removes unused latest-message bookkeeping and one duplicate unloaded-entry invocation (79 → 78 restore cases). Blox passed the full 78-case restore file and touched-file Biome on matching source. The retained mounted case still checks omission and unchanged unread state; the browser backdated-arrival poll is unchanged. Production and browser files are byte-identical to 7a47c48; the earlier 4/4 browser result is carried source evidence, not a new browser run.

At 7882e7f, entry evidence is captured by the visit owner before the first timeline mount. The delayed-arrival and read-intent cases failed before this repair; a separate synchronized-read case failed before entry attention was retained. Blox passed all 293 tests across the six focused files (83 restore cases), touched-file Biome, typecheck, and all four boundary browser cases in Chromium/WebKit with no retries. Tested source hashes match this commit. The browser fixture now verifies cold-entry omission, then re-enters loaded history while read sync remains unavailable to exercise recovery and the unchanged geometry/copy assertions. No browser cases were added or removed. The initial fixture attempt failed because optional refreshing was treated as false; it now normalizes that field explicitly. These controlled mount/sync regressions do not establish live native or multi-device behavior.

At b5b7ea3, four host-test fixtures now provide the APIs consumed by visit capture. This repairs 37 hosted-CI failures without changing production code, assertions, or test counts. Blox passed all 516 tests across the ten affected files and touched-file Biome on matching source. Production and browser files are byte-identical to 7882e7f; its 4/4 browser result remains source-matched evidence, not a new browser run.

The divider adds two browser journeys; none were removed. The original journey proves virtualized geometry, native scroll keys, and DOM selection/copy. It covers grouped rows, read progress, revisit, live appends, narrow/wide, light/dark, larger text, and keyboard/pointer scrolling. Its original <4px viewport-anchor tolerance remains unchanged. It previously passed ten repetitions per engine at 8e70de2; the current-head result above is a separate run. The new journey exercises signed backdated delivery through the production broker/store into React during read-sync recovery. Other eligibility and lifecycle cases stay in mounted React tests. Removing the copy-selection exclusion previously failed in both engines; restoring it passed. Native WebView was not exercised.

At fdcec1b, a separate synthetic Guest-channel regression confirms a read-only navigation focus defect: sidebar selection left focus outside history, so ordinary unread bold persisted. A valid membership-left notice alone did not clear it; clicking an actual message did. Both engines failed the intended history-focus assertion before the repair. With the repair, Blox passed 97/97 focused ChannelsPage and reading tests, touched-file Biome, and the entire channel-permissions browser file (10/10 in Chromium/WebKit, no retries). Tested file hashes match this commit. Typecheck also passed on the same production source.

At 8abe32d, focus intent stays tied to the original opening when a connection snapshot replaces its session-bound request. Mounted cases use the real navigation service and retain the same entry across connecting-to-error rebinding. Consumed, surrendered, and genuinely reopened cases failed before the fix; pending authority was a passing control. Blox passed all 115 tests across five focus/navigation/reading files, typecheck, touched-file Biome, and the full channel-permissions browser file (10/10 across Chromium/WebKit, no retries). No browser journeys were added or removed for this repair. The first verification attempt caught a fixture-only optional-property type error and missing browser binaries; both were repaired before this run.

The focus repair adds two browser journeys, at 1440px and 600px, and removes none. They prove actual navigation focus, layout, and the production broker/store-to-IndexedDB read receipt and bold-state wiring; mounted React cases cover delayed loading, preserved/moved focus, removed focus owners, aborted navigation, popup ownership, and one-shot handoff across remounts. Together with the divider, this PR adds four logical browser journeys, each exercised in both engines. Native WebView remains untested.

Earlier CI exposed an invalid keyboard-scroll baseline after responsive reflow. The test now starts at a known edge before measuring PageDown. Its separate reply-link failure did not reproduce in ten WebKit runs on that CI base or the full two-engine file on this candidate (14/14). The original reply-link cause remains unknown; this PR claims no reply-link fix.

The synthetic screenshots below were captured at 993af5f. The later accessibility markup does not change their visible treatment.

Before the boundary.
boundary-before

After the boundary.
boundary-after

After channel revisit, with grouped messages.
boundary-grouped

Narrow, dark view with larger text.
boundary-narrow-dark

Limits

The live #announcements marker’s cause remains unknown, even though this PR fixes an independently reproduced related read-only focus defect. Its later disappearance does not establish whether message interaction, delayed history/membership, or elapsed focus time caused it. The private screenshots do not prove the channel’s role or focus state and are not published. The synthetic membership notice did not clear unread; actual message interaction did. Local read state updates before debounced remote publication, so remote acknowledgement is a separate event. This is not a live-account or multi-device reproduction and does not claim a confirmed fix for that incident.

The divider is intentionally absent when older history remains, the first unread row was not loaded at entry, or read intent or already-known read evidence changes before capture. A visible read/unread transition cannot prove that no older unread message exists. Delayed rows cannot prove entry membership rather than a live arrival.

C1 from self-review remains a known omission, not a fix: a read change in another channel while this entry awaits history or unread evidence can suppress the divider for this visit. Entry reconciliation can also clear a prior message-force marker and advance that same revision. The public API exposes a global read-intent revision, not a target-specific entry set or mutation cause. Resetting the baseline after entry could accept intervening read changes. This PR retains the conservative guard rather than guessing or expanding the shared read-state API. Focused tests preserve these omissions and verify that the affected messages remain unread.

Coordination

Based on main, including #1963's merged held-reader resize fix. Independent of #2020, #1704, #2001, #2018, #1729, #1697, #1926, and #2024; this branch does not depend on their branches or change their ownership. It preserves #1789's relay migration and #2004's muted-channel suppression scopes.

— Generated with Codex (gpt-6.1-sol) on behalf of kylez.

@kazemek21
kazemek21 force-pushed the kylez_NEXUS-4082_channel_unread_boundary branch 5 times, most recently from 6f2edb5 to 8e70de2 Compare October 31, 2026 18:18
@kazemek21
kazemek21 marked this pull request as ready for review October 31, 2026 18:37
@kazemek21
kazemek21 requested a review from a team as a code owner October 31, 2026 18:37

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 Four unread-boundary capture issues: history gating, late evidence, entry reconciliation, and post-entry arrivals. See inline evidence and validation limits.

@kazemek21
kazemek21 force-pushed the kylez_NEXUS-4082_channel_unread_boundary branch from 8e70de2 to 42479be Compare October 31, 2026 19:24
@kazemek21

Copy link
Copy Markdown
Contributor Author

Disposition of the four-point review at 42479be:

All four threads have attributed answers and invite reopening. Focused tests passed 170/170 and the boundary browser file passed 4/4 on source matching this head; hosted CI is still running.

— Posted by Codex (gpt-6.1-sol) on behalf of kylez

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

Reviewed head 42479be7fb243288128be3bbee774afddda4dffc against base c253fc051e1aa7acb4c6c7f2f64f8b779e2a41cd.

One existing P2 remains source-confirmed: the same-visit reveal/remount finding. ConversationTab.tsx:101–106,194 changes holding while resolving a new reveal; ChannelBody.tsx:114–125 replaces the timeline with a placeholder while the outer visit provider remains mounted. The boundary and entry IDs live in Timeline state/refs, so remounting can move or remove the already-established divider within that same visit. Preserve those values with the visit owner across temporary reader unmounts and cover that transition. Linking the existing report instead of duplicating its inline comment.

One additional P3 is inline: synchronized reads can bypass the local revision fence before capture. This affects the entry annotation, not stored read state. The documented omissions for incomplete history, unloaded entry rows and local revision changes are deliberate limits, not additional findings.

Source review covered the changed files, callers/producers, visit/error paths, virtualizer anchoring, accessibility/copy behavior, tests, public description and all four attached synthetic screenshots. No concrete public-material disclosure found. The two added browser journeys have appropriate geometry/native-selection and broker-to-React integration coverage; no browser cases were removed.

Validation: no code execution, tests or app launches by this review. Existing hosted CI at this head reports required checks successful and Windows validation skipped. Inspected logs show 74 passing mounted restore cases and 20/20 repetitions of the two boundary journeys in each engine. Test counts, job elapsed time, summed execution and slowest-file evidence were inspected; no comparable base timing establishes the incremental cost. Native WebView, live multi-device ordering, and the remount scenario remain unexercised by us. This is a COMMENT review, not approval.

@kazemek21
kazemek21 force-pushed the kylez_NEXUS-4082_channel_unread_boundary branch from 42479be to 7a47c48 Compare November 1, 2026 03:35
@kazemek21

Copy link
Copy Markdown
Contributor Author

Both findings in the source review are fixed at 7a47c48:

  • Same-visit reveal/remount: retain completed boundaries and pending baselines under the existing visit token, including a captured empty result.
  • Synchronized read ordering: detect changes in already-known entry attention without relying on the local-intent revision, and omit the uncertain divider instead of shifting it.

All five new mounted regressions failed on the prior head and pass after repair. Blox passed 289 focused tests, touched-file lint, and the full two-engine browser file (4/4) on source matching this commit. Hosted checks are still running. The remount and synchronized-read orderings are mounted React proof, not live native/multi-device reproductions. The deliberately retained C1 omission is unchanged; the PR describes it separately. Both threads now have attributed answers and invite reopening.

— Posted by Codex (gpt-6.1-sol) on behalf of kylez

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 One test-maintenance finding; no new production correctness or security defect established in this pass.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

Reviewed head 7a47c48a6c69f041f84d32ff45d5fbd631c7cd82 against base c014f54b33f6003612c3b1f2e6314354bdcb514d, focusing on changes since reviewed head 42479be7fb243288128be3bbee774afddda4dffc.

No new actionable findings. Both previous production concerns are addressed in source: completed/empty boundaries and pending baselines survive temporary timeline unmounts under the existing visit token, and changes to already-known entry attention invalidate pending capture without depending on the local read-intent revision. The five added mounted cases cover these orderings. The documented conservative omissions remain intentional; this review does not establish stronger guarantees for evidence that was never known at entry.

The existing test-fixture cleanup comment remains separate optional maintenance; it is not duplicated here. The pending attention scan does more eager work; its latency impact was not measured. No browser journeys were added or removed in this follow-up.

Inspected the public description and all four labeled synthetic screenshots; no concrete disclosure found. Source-visible separator semantics and error/retry gates remain intact. The screenshots represent 993af5f, not a fresh render of this head.

Validation: source-only; no tests, builds or app execution by this review. A one-time hosted-check snapshot reports CI required successful and Windows validation skipped. The author’s detailed test counts were not independently reproduced. Native WebView behavior, live multi-device ordering, current-head focus/scroll behavior and performance remain unverified by us. This is a COMMENT review, not approval.

@kazemek21
kazemek21 force-pushed the kylez_NEXUS-4082_channel_unread_boundary branch from 7a47c48 to 7abf46d Compare November 1, 2026 04:08
@kazemek21

Copy link
Copy Markdown
Contributor Author

The test-maintenance review is addressed in 7abf46d: obsolete latest-message fixture bookkeeping and one equivalent invocation were removed, retaining no-divider/unchanged-unread assertions and the browser readiness poll. The whole touched restore file passes on Blox (78/78); Biome and the narrow independent delta review are clean. Hosted CI is pending. On-thread disposition.

— Posted by Codex (gpt-6.1-sol) on behalf of kylez

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

Reviewed head 7abf46d37cf0eb671b3e0be94558ae32559bea56 against base c014f54b33f6003612c3b1f2e6314354bdcb514d, focusing on the fixture cleanup since reviewed head 7a47c48a6c69f041f84d32ff45d5fbd631c7cd82.

No new actionable findings. The endpoint comparison changes only ChannelTimeline.restore.test.tsx (+13/-38); production and browser files are unchanged. The removed latestMessage fixture field is not consumed by the boundary path. Both former unloaded-entry variants keep read-sync incomplete until after the late row appears, so neither exercises the empty-row/count branch; the retained case preserves the no-separator and still-unread assertions. Deferred entry/snapshot ordering and fixture teardown remain intact. No browser cases were added or removed in this follow-up.

Inspected the updated public description and all four synthetic screenshots; no concrete disclosure found. The screenshots remain from 993af5f, not a new rendering of this head.

Validation: source-only; no tests, builds or app execution by this review. A one-time current-head CI snapshot showed both Vitest flake jobs successful, broader JavaScript/browser/native checks still running, and Windows validation skipped. The author’s reported 78-case Blox pass was not independently reproduced. Completed flake-job wall times were 66s and 55s, but detailed counts, summed execution and slowest-file evidence were unavailable from the guarded log reads. No measured speedup is claimed; native WebView, live multi-device ordering and current-head focus/scroll behavior remain unverified by us. This is a COMMENT review, not approval.

@kazemek21
kazemek21 force-pushed the kylez_NEXUS-4082_channel_unread_boundary branch from 7abf46d to 7882e7f Compare November 1, 2026 04:56

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

Reviewed head 7882e7f against base 7878f06, focusing on the actual endpoint changes since 7abf46d.

No new actionable findings. Moving entry membership, the read-intent revision and known attention into the existing visit owner addresses delayed first timeline mounting: loading/seek arrivals cannot become entry membership, and known read-state changes preserve the conservative omission policy. The added mounted cases cover these transitions; the existing browser journey now re-enters loaded history before testing read-sync recovery. No browser cases were added or removed in this follow-up.

Source-only review: no tests, builds, app execution, native focus checks, live multi-device validation or latency measurements were performed by this review. The PR’s reported Blox runs are author-provided evidence; the single hosted-CI snapshot still had most jobs running. Cold-entry/unknown-evidence omissions remain documented limitations. Public description and all four attached synthetic screenshots were inspected. This is a COMMENT review, not approval or merge authorization.

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 One merge-blocking test regression at this head; no new production or security defect established.

@kazemek21
kazemek21 force-pushed the kylez_NEXUS-4082_channel_unread_boundary branch from 7882e7f to b5b7ea3 Compare November 1, 2026 05:11
@kazemek21

Copy link
Copy Markdown
Contributor Author

The fixture regression in the review is fixed in b5b7ea3. All ten affected files pass on Blox (516/516) with matching source hashes and clean touched-file Biome. Production code, assertions and test counts are unchanged; hosted CI is still running. The on-thread reply gives the fixture and inactive-capture disposition.

— Posted by Codex (gpt-6.1-sol) on behalf of kylez

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated source review via Wes’s account: no new actionable findings in this follow-up.

Compared with reviewed revision 7882e7f, the only changes are four host-test fixtures (+79/−14). They supply the channel window, unread revision and sync APIs now consumed by visit capture. Existing assertions and test counts are preserved; production and browser files are unchanged. This uses the small-follow-up exception to the prior four-Guardian review. The public description and all four attached screenshots were also inspected.

Head: b5b7ea3
Base: 7878f06

Source-only: no tests, builds or app execution performed in this review. One hosted-check snapshot showed CI required and the listed JS/browser/flake checks successful; Windows validation was skipped. This does not establish live native, multi-device or human acceptance, and is not a GitHub approval.

@kazemek21
kazemek21 force-pushed the kylez_NEXUS-4082_channel_unread_boundary branch from b5b7ea3 to fdcec1b Compare November 1, 2026 15:19
@kazemek21 kazemek21 changed the title NEXUS-4082: Show a stable new-message boundary NEXUS-4082: Stabilize new-message boundaries and read-only focus Nov 1, 2026

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

Reviewed head fdcec1b189bea7f2ff212f8293859ca37afa8a53 against base 923a80f4aeed4ab7a671c4df62f10b3a81bf15bc, focusing on the four-file change since reviewed head b5b7ea32e9d48c13654cd47ccb89de04f39c1f5d. One P2 finding is detailed inline. The earlier unread-boundary implementation is unchanged in this follow-up.

The two added browser journeys cover wide/narrow navigation focus and durable read-state wiring; ownership permutations remain in mounted component tests, and no browser journeys were removed. Reviewed the public description and all four attached synthetic screenshots.

Source-only: no tests, builds, app launches or native/live-account workflows were executed. The PR’s reported Blox results are author-provided evidence, not runs performed by this review. Error/retry focus and native WebView behavior remain unexercised here. The single hosted-check snapshot at approximately 15:29 UTC still had browser jobs running and Windows native validation skipped; it does not establish a green final pipeline or runtime acceptance.

Comment thread src/bundled/channels/ChannelsPage.tsx Resolved
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Kyle Zemek <kylez@squareup.com>

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 The existing P2 focus-rebinding finding is reproduced at fdcec1b in Chromium and WebKit: populate Guest history/cache, reload an ordinary channel with the session handshake held, explicitly open the cached Guest channel, focus Search Buzz, then advance the frontend’s eight-second connection timeout. Both engines move focus back to history; direct cached reload without the explicit opening passes as a control. A mounted same-entry rebinding test reproduces it as well.

Preserve consumed/cancelled focus intent across bound-request replacement for the same opening. Fix this before merge; no additional actionable findings. Reusing the existing inline anchor to avoid duplicating the finding.

@kazemek21
kazemek21 force-pushed the kylez_NEXUS-4082_channel_unread_boundary branch from fdcec1b to 8abe32d Compare November 1, 2026 15:48
@kazemek21

Copy link
Copy Markdown
Contributor Author

The same-opening focus-rebinding issue in Wes’s review and Kalvin’s independent reproduction is fixed in 8abe32d. The inline reply explains the opening/request distinction and regression evidence. Blox passed typecheck, touched-file lint, 115 targeted tests, and all 10 channel-permissions cases in Chromium and WebKit; hosted CI is still running.

This fixes the independently established synthetic focus defect, not a proven cause of the original real-account announcements symptom. Native WebView behavior remains unverified. Please reopen the thread if the fix does not address the concern.

— Posted by Codex (gpt-6.1-sol) on behalf of kylez

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

No new actionable findings. The prior P2 focus-rearming defect is addressed in source: the original opening owns the focus intent, while session rebinding updates only its authority. Consumed or surrendered intent stays consumed; a genuinely new opening can focus history again. The four mounted cases use the real navigation service to cover consumed, surrendered, pending and reopened intent across connecting-to-error rebinding. No browser journeys were added or removed in this follow-up.

Reviewed head 8abe32d0e922cfc52f5ddb7f0ab4dcffb4de8440 against base ca836a3109be30a7fd46db97bdcceaf32ebabf65, focusing on the repair since fdcec1b189bea7f2ff212f8293859ca37afa8a53 and separating inherited base changes. Reviewed the public description and all four attached synthetic screenshots.

Source-only: no tests, builds or app execution by this review. The PR’s reported Blox results are author-provided evidence. Our single hosted-check snapshot still had browser jobs running and Windows native validation skipped. Native/live-account behavior, actual error/retry focus, live multi-device ordering and performance remain unverified by us. This is a COMMENT review, not approval.

@kazemek21
kazemek21 requested a review from kalvinnchau November 1, 2026 16:02
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.

4 participants