Fix mobile Activity thread navigation - #5850
Conversation
![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: 9362c2d6d0
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 109f0ce8d6
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ba459935b
ℹ️ 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".
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Reviewed exact head 59966c61281473d2602aeb782a55fdbd01e2b644.
Request changes
Major: make the Activity landing target perceivable and available nonvisually
mobile/lib/features/channels/thread_detail_page.dart:43-46,794-825 identifies the referenced reply only by briefly blending the primary color into its background at 0.12 * 0.4 = 4.8% alpha. Against the repository's light/dark surfaces, that produces roughly 1.04:1 / 1.10:1 adjacent contrast. There is also no semantics announcement, focus movement, label/state change, outline, or other non-color cue.
This is the orientation affordance for the feature: after Activity opens a long thread around 35% viewport alignment, users must be able to identify which nearby reply was referenced. At this contrast it is effectively imperceptible, and screen-reader users receive no target indication at all. The tests at mobile/test/features/channels/channel_detail_page_test.dart:4122-4133,4275-4314 assert the 0.048 alpha, so they preserve the defect rather than proving the target is recognizable.
Use a clearly distinguishable state treatment (for example, a sufficiently contrasted border or leading marker rather than only a faint wash) and announce/focus the target without trapping reading navigation. Preserve reduced-motion behavior. Add semantics assertions and light/dark rendered evidence.
Validation and remaining gate
The route/state implementation otherwise held up: replacement is Activity-only, delayed hydration refuses to replace a newer route, the thread retains visible-channel ownership, delayed targets wait for layout, and Back returns directly to Activity. Focused route/highlight tests passed, as did the full 1,359-test Flutter suite reported at this head. My exact-head just mobile-check passed (394 files formatted/0 changed, no analyzer issues, file-size guard), and CI Mobile is green.
However, GitHub currently reports this head as DIRTY / CONFLICTING against main. Resolve the conflict, then the new SHA needs delta review and affected gates rerun. Please also supply real Activity → delayed/paginated thread → target identification → Back evidence, including screen-reader/semantics behavior; the current tests split the journey across synthetic entry points and the PR supplies no device artifact for it.
Non-blocking scope note: the pull-to-refresh bee eye changes in mobile/lib/shared/widgets/bee_refresh_indicator.dart, flapping_bee.dart, and their test are unrelated to Activity navigation and should preferably be split into their own PR.
Signed-off-by: kenny lopez <klopez4212@gmail.com>
Signed-off-by: kenny lopez <klopez4212@gmail.com>
Co-authored-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz> Signed-off-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz>
Co-authored-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz> Signed-off-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz>
Co-authored-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz> Signed-off-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz>
59966c6 to
af0d985
Compare
|
@codex review — Princess Donut |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af0d9854d5
ℹ️ 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".
Co-authored-by: Kenny Lopez <klopez4212@gmail.com> Signed-off-by: Kenny Lopez <klopez4212@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e36493704
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Reviewed exact head 6e36493704a0d4d965654d65c2a25975bff2e149 with the Royal Court.
Request changes
Major: make the Activity landing target perceivable visually and nonvisually
The prior target-identification blocker remains unresolved. mobile/lib/features/channels/thread_detail_page.dart:49-52,142-199 makes the landing state transient and caps its only visual treatment at 0.12 * 0.4 = 4.8% alpha. mobile/lib/features/channels/thread_detail_page/thread_message.dart:85-117 renders only that background-color wash. Against the repository’s light and dark surfaces this is approximately 1.07:1 adjacent contrast, with no border, marker, focus movement, semantics announcement, label/state change, or other non-color cue.
That means Activity can correctly position a long thread around the target while leaving sighted users unable to distinguish which nearby message was referenced, and VoiceOver/TalkBack users receive no target identification at all. Reduced motion snaps to the same inaccessible state rather than solving it.
Please add a clearly distinguishable target treatment, such as a contrast-safe border or leading marker, and announce or appropriately focus the target without trapping reading navigation. Preserve the reduced-motion behavior. Add semantics assertions and light/dark rendered evidence; the changed tests at mobile/test/features/channels/channel_detail_page_test.dart:4277-4533,4584-4629 currently assert the raw 4.8%-alpha decoration and timing, so they encode the defect rather than proving recognizability.
Other review results
The navigation/state-machine seams otherwise held up: Activity-only replacement preserves Back, delayed hydration cannot replace a newer route, the thread retains visible-channel ownership, target selection waits for authoritative pagination or terminal failure, and listener/timer disposal is guarded. The Royal Court reached consensus on this verdict.
Focused exact-head tests passed for Activity route wiring, replacement/Back, delayed-hydration cancellation, and query-failure fallback. git diff --check is clean, and exact-head Mobile CI is green. Two initially parallel local test invocations collided in Flutter’s shared asset directory; rerunning the affected tests serially passed.
Scope note, non-blocking: the pull-to-refresh bee eye changes and their tests are unrelated to Activity thread navigation and would be cleaner in a separate PR.
|
Carl, an automated reviewer, commenting via Wes’s GitHub account. Exact-head follow-up: changes remain requested at The prior major target-identification blocker remains unresolved:
Please add a clearly distinguishable treatment, such as a contrast-safe border or leading marker, and announce or appropriately focus the target without trapping reading navigation. Preserve reduced-motion behavior and add semantics plus light/dark rendered coverage. The Royal Court reached consensus. Navigation/state-machine review otherwise found no material issue: Activity-only replacement preserves Back, delayed hydration cannot replace a newer route, visible-channel ownership survives replacement, target selection waits for pagination or terminal query failure, and listener/timer cleanup is guarded. Focused exact-head tests passed for Activity route wiring, replacement/Back, delayed-hydration cancellation, and query-failure fallback. Non-blocking scope note: the pull-to-refresh bee eye changes and tests are unrelated and would be cleaner in a separate PR. |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent reviewed exact head 6e36493704a0d4d965654d65c2a25975bff2e149 against base f956e6fe06a76e50cbd8fba1a162482e752e7f1a.
Request changes
P2 — a retrying thread query is treated as terminal and consumes the one-shot jump
mobile/lib/features/channels/thread_detail_page.dart:116-121,350-396 says the hydrated route snapshot is eligible only after a terminal query error, but implements that as replyMessages.hasError. Under the pinned Riverpod 3.1.0, a failed provider with a retry scheduled is AsyncLoading carrying both hasError == true and retrying == true; the default container retries ordinary exceptions up to ten times. The first transient relay failure therefore authorizes the provisional snapshot immediately and sets didJumpToInitialMessage at lines 366-367. If a retry later succeeds with the authoritative/paginated list and a different target index, the latch prevents correction.
The new regression test does not exercise production semantics: mobile/test/features/channels/channel_detail_page_test.dart:196-207,4474-4533 explicitly sets disableRetries: true, converting the first failure into a terminal error. It passes while the retry/recovery path remains broken.
Please require an authoritative value or a non-retrying error, for example fetchedReplies != null || (replyMessages.hasError && !replyMessages.retrying). Add a production-retry test where the first attempt fails, no jump/highlight occurs while retrying, and a retry succeeds with an authoritative list whose target index differs. Mutation-prove that removing the !retrying guard fails the test.
P2 — the Activity landing target remains imperceptible visually and unidentified nonvisually
mobile/lib/features/channels/thread_detail_page.dart:49-52,142-199 makes the target state transient and caps its only treatment at 0.12 * 0.4 = 4.8% alpha. mobile/lib/features/channels/thread_detail_page/thread_message.dart:85-117 renders only that background wash—no border, marker, focus movement, announcement, or target semantics. With the checked-in default colors, the resulting adjacent contrast is about 1.076:1 light (#F9F5FE on #FFFFFF) and 1.096:1 dark (#2C2D43 on #24273A). Users can land among nearby replies without being able to tell which was referenced; VoiceOver/TalkBack receives no identification. That conflicts with VISION.md:206's WCAG 2.1 AA minimum.
The changed rows at mobile/test/features/channels/channel_detail_page_test.dart:4277-4533,4584-4629 assert raw alpha/timing, not recognizability or semantics. Please add a clear non-color cue such as a contrast-safe border/leading marker plus a one-shot announcement or appropriate focus behavior that does not trap reading navigation. Preserve reduced-motion behavior and add semantics assertions plus light/dark rendered/native evidence.
Integrated validation
At exact clean head:
git diff --check f956e6fe06a76e50cbd8fba1a162482e752e7f1a...HEAD— pass. ./bin/activate-hermit && just mobile-check— pass; 409 files formatted/0 changed, analyzer cleancd mobile && flutter test test/features/activity/activity_page_test.dart— pass, 25 testscd mobile && flutter test test/features/channels/channel_detail_page_test.dart --plain-name 'highlights a hydrated target after the thread query fails'— pass, but only with retries disabled by the test- GitHub Mobile and DCO checks — pass at this exact head
The navigation/state and ownership paths otherwise held up: Activity-only replacement preserves Back, stale hydration is guarded, visible-channel registration is disposed, and no persistence/security/tenant boundary changes were introduced. No real iOS/Android Activity → delayed/paginated target → Back plus VoiceOver/TalkBack artifact was supplied, so the user-visible workflow remains unproven outside widget tests.
Non-blocking scope note: the pull-to-refresh bee-eye source and test changes are unrelated to Activity navigation and should be split.
Co-authored-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz> Signed-off-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz>
|
Addressed the non-UI retry correctness finding in Riverpod 3.1 represents a scheduled retry as The requested visible marker/semantics treatment would change the authored UI, so I intentionally made no accessibility/UI change while awaiting the author’s confirmation. Validation at exact pushed head: — Princess Donut |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8aea947592
ℹ️ 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".
Co-authored-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz> Signed-off-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz>
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent reviewed exact head 6f146fdba0c41c9dfe05b64d0c479bee5b9c1ed6 against base f956e6fe06a76e50cbd8fba1a162482e752e7f1a.
Request changes
P2 — the Activity target remains color-only and unidentified to assistive technology
mobile/lib/features/channels/thread_detail_page.dart:49-52,140-199 still makes the landing treatment a transient primary-color wash capped at 0.12 * 0.4 = 4.8% alpha. mobile/lib/features/channels/thread_detail_page/thread_message.dart:85-117 renders only that background color: there is no border/marker, target semantics, announcement, or focus behavior. The resulting adjacent contrast remains approximately 1.07:1, and VoiceOver/TalkBack receives no target identification.
Activity can now land at the correct reply among nearby rows, but sighted users can miss which reply was referenced and screen-reader users are not told at all. This defeats the feature’s orientation affordance and conflicts with the repository’s WCAG 2.1 AA requirement.
Please add a persistent-enough non-color cue such as a contrast-safe leading marker or border, plus a one-shot semantics announcement or carefully managed focus that does not trap reading navigation. Preserve reduced-motion behavior. Add semantics assertions and light/dark rendered coverage, and mutation-prove removal of each cue.
Corrective delta and integrated validation
The retry correction is sound. thread_detail_page.dart:95-123,352-429 now separates authoritative reply readiness from the rendered relay+local overlay, opens the one-shot jump gate only on an authoritative value or hasError && !retrying, and consumes the latch only after a valid final index. Existing Activity replacement/Back ownership, stale-navigation guards, target layout sequencing, and visible-channel cleanup remain coherent. No persistence, auth, security, or community-scoping surface changed.
At the exact clean head:
git diff --check f956e6fe...HEAD— passjust mobile-check— pass; 409 files formatted/0 changed, analyzer and file-size guard cleanflutter test test/features/channels/channel_detail_page_test.dart— pass, 136/136flutter test test/features/activity/activity_page_test.dart— pass, 25/25- Production-retry/local-overlay regression — pass; restoring the pre-fix merged-state gate causally fails at
channel_detail_page_test.dart:4571before restoration and green rerun - GitHub Mobile, DCO, and all executed required checks — success; Web skipped by path detection
No exact-SHA native Activity → delayed/paginated target → Back artifact or VoiceOver/TalkBack evidence was supplied. Widget evidence establishes the state machine, not real-device target recognizability or screen-reader behavior.
The retry/recovery blocker is resolved. The unchanged visual/nonvisual target-identification defect remains blocking.
|
🤖 Additive review notes from an independent pass at head The retry and local-overlay correction is real, and I confirmed it under mutation. The gate now reads the authoritative relay provider and admits only on a value or on a terminal error with retry finished, rather than the merged relay-plus-local value whose loading and error branches both return Contrast, recomputed with a calibrated instrument. Controls: white on black 21.0000,
The merge base The result does not depend on the theme or the accent, which is the part I think is genuinely new. One number already on this PR is wrong, and here is why. The dark figure of about 1.096 with composite The five changed test rows hardcode the literal, and they pin it in both directions. Nothing under
The practical consequence: the rows are not a loose guard, they are an exact pin on 0.048, and any change to the perceptibility of this cue reds 4 of 7 tests across 4 test cases, three of them in rows that read as "still fading in" rather than "the peak is 0.048". That is worth knowing before someone estimates the work as a one-line constant change. One thing that is not this PR's to owe. The screen-reader half of the concern is pre-existing and app-wide, not introduced here. There is no Bounds on all of the above. The contrast and sweep figures are computed composites, not rendered artifacts, so they show the numbers are low but not where a human recognition threshold sits. The mutation claims are scoped to Separately, and not blocking: pull-to-refresh bee-eye rendering changed here too. It is behaviorally independent of Activity routing, target selection and route ownership. |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent re-adjudicated exact head 6f146fdba0c41c9dfe05b64d0c479bee5b9c1ed6 against merge base f956e6fe06a76e50cbd8fba1a162482e752e7f1a, including the author’s corrected measurements and mutation evidence.
Request changes — narrowed finding
P2 — this PR weakens its only target-orientation cue from persistent 12% to transient 4.8%
The accessibility concern needs a scope correction, but the visual regression remains material.
- Merge base and current
mainhave identical relevant blobs. Basemobile/lib/features/channels/thread_detail_page/thread_message.dart:85-94appliesprimaryat 12% whenever the initial message matches, and basethread_detail_page.dart:632-633,713keeps that state for the route lifetime. - Candidate
thread_detail_page.dart:49-52,140-199changes the peak to0.12 * 0.4= 4.8%, delays it until navigation settles, holds it for three seconds, then removes it. Candidatethread_message.dart:85-117leaves that wash as the only visible target identifier. - Recomputed from checked-in colors, light changes from 1.1939:1 to 1.0758:1 and dark from 1.1811:1 to 1.0692:1. The earlier ~1.096 dark figure used the wrong mauve; the author’s corrected 1.0692 figure is right. Sweeping the shipped selectable accents does not rescue a 4.8% overlay.
This is transient orientation/status feedback after programmatic navigation, not decorative color: Activity positions a referenced reply among neighboring rows, and this cue identifies which row was targeted. The PR owns the new settle/fade/hold contract and reduces its sole visual signal by 60% while making it transient. The baseline cue was already weak, but that does not make a concrete regression in the changed contract acceptable without rendered/user evidence that the new peak remains recognizable.
Scope and smallest remedy
The nonvisual gap predates this PR. Base/current mobile/lib has no target-specific announcement or focus behavior, and requiring a new app-wide announcement system here would exceed the regression boundary. Flutter Semantics(liveRegion: true, ...) already exists locally elsewhere in the app, so a localized semantics improvement is possible and recommended, but it is not the independent merge gate in this adjudication.
The smallest contract-preserving repair is local: restore at least the pre-existing 0.12 peak while preserving the corrected navigation, retry, route-settle, fade, hold, and reduced-motion behavior, then update the coupled expectations. A stronger locally measured non-color marker/border would better satisfy the state-identification requirement, but no global primitive is required.
Mutation evidence establishes that this remedy is bounded: changing only the production peak to 0.12 fails four of seven Deep-link rows because five expectations pin 0.12 * 0.4; updating those expectations to 0.12 makes the complete group pass 7/7 without changing the state machine. Thus the current tests defend the weaker value rather than proving recognizability.
What clears this finding
A new immutable head that restores at least the pre-existing 0.12 peak, updates causal tests, retains the current state-machine behavior, and passes affected exact-head gates. Alternatively, same-host rendered/native evidence demonstrating reliable target recognition at 0.048 across shipped light/dark/accent combinations could rebut materiality. A localized ≥3:1 non-color cue with light/dark/accent rendered coverage would be the stronger fix; app-wide announcement infrastructure is explicitly not required.
The retry/local-overlay correction, Activity replacement/Back ownership, stale-navigation guards, and lifecycle cleanup remain clear. Prior exact-head just mobile-check, channel-detail 136/136, Activity 25/25, retry mutation, Mobile CI, and DCO evidence remain valid because the head is unchanged.
Reviewers flagged that the Activity landing highlight peak had been reduced to 0.12 * 0.4 (4.8% alpha), well below the persistent 0.12 highlight on main and below the target's WCAG-recognizability threshold. Restore the peak to 0.12 while preserving the settle/fade/hold/reduced-motion contract, and update the five test rows that pinned 0.12 * 0.4 to 0.12. Signed-off-by: Kenny Lopez <klopez4212@gmail.com> Co-authored-by: Mongo <9cfd347903944d5b85aa6c93d2ab67381b978a92a31914bca69998968752a1d7@buzz.block.builderlab.xyz> Co-authored-by: Kenny Lopez <klopez4212@gmail.com>
…ivity-thread-navigation Signed-off-by: Kenny Lopez <klopez4212@gmail.com>
|
Restored the Activity landing highlight peak to 0.12 in The highlight peak had been reduced to The five test rows in Validation at the merged tree:
This is the smallest contract-preserving remedy Jude spelled out. The nonvisual/semantics gap remains pre-existing and app-wide (no |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent re-reviewed exact head 5e1d92ca82345094137f328775717cc4f8d36c61 against base/merge-base 417eea2230c1864e8c77f6440dbcfa109bfb63f6.
Code verdict: no remaining finding in the mobile change. Merge gate: not clear while required CI is red.
The implementation now preserves the intended state ownership:
- Activity opts into replacing only its temporary channel route, and delayed hydration first verifies that route is still current before replacing it (
mobile/lib/features/activity/activity_page.dart:217-247,mobile/lib/features/channels/channel_detail_page/message_list.dart:560-593). Back therefore returns to Activity rather than the hydration route, while a newer route cannot be displaced. - The initial target jump waits for authoritative relay replies or a terminal, non-retrying query error; it consumes the one-shot latch only after finding the target and waits for layout before revealing the cue (
mobile/lib/features/channels/thread_detail_page.dart:117-123,352-429). - The corrective commit restores the landing highlight peak to the existing
0.12baseline without changing the route-settle, fade, three-second hold, reduced-motion, timer-cleanup, or visible-channel ownership contracts (mobile/lib/features/channels/thread_detail_page.dart:49-52,87-90,140-201;mobile/lib/features/channels/thread_detail_page/thread_message.dart:85-108). Regression tests cover delayed hydration, provider retry, terminal failure fallback, stale-route rejection, Back ownership, visible-channel cleanup, and the exact opacity peak (mobile/test/features/channels/channel_detail_page_test.dart:4451-5015). Mutation-checking the prior0.048value made the two peak assertions fail; restoring the exact head returned the deep-link group to green.
Exact-head evidence:
git diff --check 417eea2230c1864e8c77f6440dbcfa109bfb63f6...HEAD: pass.just mobile-check: pass (format, analyzer, file-size guard).mobile/test/features/channels/channel_detail_page_test.dart: 137/137 pass.- Activity tests: 25/25 pass.
- GitHub Mobile and DCO checks: pass.
- The PR head remains
5e1d92ca82345094137f328775717cc4f8d36c61and GitHub reports it mergeable.
The current required Desktop Core check failed in managed_agents::discovery::tests::probe_codex_acp_version_returns_version_when_descendant_holds_pipe_open (None versus Some((1, 1, 2)); 2513 passed, 1 failed, 17 ignored). That code arrived from the updated base and is outside this PR's base-relative ten-file mobile diff, but a failed required check still needs a clean rerun or an explicit maintainer disposition before merge. The aggregate Desktop check is red only because that job is red.
Residual risk: no exact-head native iOS/Android Activity → delayed or paginated target → Back run, nor VoiceOver/TalkBack pass, was supplied. Widget coverage establishes the state machine and reduced-motion behavior, not real-device perceptibility. The color-only target cue predates this change; this head restores rather than weakens its prior opacity, so it is not a new blocker for this bounded fix.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Reviewed exact head 5e1d92ca82345094137f328775717cc4f8d36c61 against merge base 417eea2230c1864e8c77f6440dbcfa109bfb63f6 with the Royal Court.
Verdict: code clear; required CI is not
No material code finding remains in the mobile delta. The Court reached consensus that the implementation now preserves the relevant contracts:
- Activity opts into replacing only its temporary hydrated channel route, preserving Activity as the Back destination. Delayed hydration verifies that route is still current before replacing it, so a stale callback cannot displace newer navigation.
- The target jump waits for authoritative relay replies or terminal non-retrying failure, does not consume its one-shot latch until a valid target exists, and waits for layout and route settlement before revealing the cue.
- Visible-channel ownership, route listeners, and highlight timers are cleaned up correctly.
- The corrective head restores the pre-existing
0.12highlight peak while preserving fade-in, three-second hold, fade-out, and reduced-motion behavior. - The unrelated bee-refresh eye-mask correction is small and causally tested, though disclosing it in the PR description would improve scope hygiene.
Exact-head evidence includes clean git diff --check, prior exact-head just mobile-check, channel-detail tests 137/137, Activity tests 25/25, and green GitHub Mobile, DCO, macOS build, smoke, and integration checks.
Merge gate
The required Desktop Core check remains red. Its only failure is the out-of-scope base-code test managed_agents::discovery::tests::probe_codex_acp_version_returns_version_when_descendant_holds_pipe_open (None versus Some((1, 1, 2)); 2,513 passed, 1 failed). The same test passed earlier in that job, which supports flakiness, but a required failed check still needs a clean rerun or explicit maintainer disposition before merge. Aggregate Desktop is red because of that job.
Residual risk: no exact-head native iOS/Android Activity → delayed or paginated target → Back run, nor VoiceOver/TalkBack pass, was supplied. Widget coverage establishes the state machine, not device-level behavior. The color-only target cue predates this PR; this head restores rather than weakens its baseline opacity, so it is not a bounded-diff blocker.
…-in-thread * origin/main: (32 commits) Revert "fix(acp): gate relay-signed workflow messages on their attributed author" (#6311) fix(desktop): morph the drawer panel icon instead of sliding it (#6306) feat(desktop): refine repository-aware project workspaces (#6003) Fix mobile Activity thread navigation (#5850) perf(desktop): parallelize relay agent directory rebuild (#6258) Refine the mobile emoji picker (#5853) fix(desktop): exclude archived agents from nest, order regeneration (#5905) Add font size and conversation density preferences (#5644) fix(desktop): emit camelCase config-write payload fields (#6062) fix(desktop): downscale large avatars for agent-share PNG body (#6260) fix(desktop): preserve early relay auth challenges (#3320) Polish mobile message actions (#5873) Refine mobile pairing confirmation (#6018) chore(scripts): add buzz-adopt-prod-agents.sh (#6250) feat(managed-agents): close five Claude Code agent-config gaps (#4557) chore(hooks): keep mobile analysis out of pre-commit (#6236) fix(shared-ui): delay hover disclosures by default (#5821) fix(desktop-chrome): preserve balanced layout when sidebar collapses (#6000) Polish mobile timeline navigation (#5874) chore(release): release Buzz Desktop version 0.5.17 (#6234) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…c-agent-commit-identity * origin/main: Revert "fix(acp): gate relay-signed workflow messages on their attributed author" (#6311) fix(desktop): morph the drawer panel icon instead of sliding it (#6306) feat(desktop): refine repository-aware project workspaces (#6003) Fix mobile Activity thread navigation (#5850) perf(desktop): parallelize relay agent directory rebuild (#6258) Refine the mobile emoji picker (#5853) fix(desktop): exclude archived agents from nest, order regeneration (#5905) Add font size and conversation density preferences (#5644) fix(desktop): emit camelCase config-write payload fields (#6062) fix(desktop): downscale large avatars for agent-share PNG body (#6260) fix(desktop): preserve early relay auth challenges (#3320) Polish mobile message actions (#5873) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…ntion-phase1 * origin/main: (71 commits) Revert "fix(acp): gate relay-signed workflow messages on their attributed author" (#6311) fix(desktop): morph the drawer panel icon instead of sliding it (#6306) feat(desktop): refine repository-aware project workspaces (#6003) Fix mobile Activity thread navigation (#5850) perf(desktop): parallelize relay agent directory rebuild (#6258) Refine the mobile emoji picker (#5853) fix(desktop): exclude archived agents from nest, order regeneration (#5905) Add font size and conversation density preferences (#5644) fix(desktop): emit camelCase config-write payload fields (#6062) fix(desktop): downscale large avatars for agent-share PNG body (#6260) fix(desktop): preserve early relay auth challenges (#3320) Polish mobile message actions (#5873) Refine mobile pairing confirmation (#6018) chore(scripts): add buzz-adopt-prod-agents.sh (#6250) feat(managed-agents): close five Claude Code agent-config gaps (#4557) chore(hooks): keep mobile analysis out of pre-commit (#6236) fix(shared-ui): delay hover disclosures by default (#5821) fix(desktop-chrome): preserve balanced layout when sidebar collapses (#6000) Polish mobile timeline navigation (#5874) chore(release): release Buzz Desktop version 0.5.17 (#6234) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…urneys * origin/main: chore: serialize mobile pre-push checks (block#6322) fix(buzz-acp): loosen workspace-scan guardrail to allow named paths (block#6261) fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths (block#6271) perf(desktop): move five hot renderer paths from JS into Rust (block#6024) fix(media): accept portrait video resolutions (block#6058) fix(desktop): hide archived channels from #/Tab autocomplete (block#6156) Unify mobile channel details (block#6113) Revert "fix(acp): gate relay-signed workflow messages on their attributed author" (block#6311) fix(desktop): morph the drawer panel icon instead of sliding it (block#6306) feat(desktop): refine repository-aware project workspaces (block#6003) Fix mobile Activity thread navigation (block#5850) Signed-off-by: Jude Edwards <judeedwards@squareup.com>
4 participants
Summary
Why
Activity deep links hydrated the channel before opening the thread, which left the temporary channel route in the navigation stack. The target highlight also appeared before the route settled and remained indefinitely.
Validation
just mobile-check