Skip to content

Complete Activity attention queue and exact-source navigation - #1588

Merged
lilyshen0722 merged 36 commits into
mainfrom
feat/pr3-activity-contract
Sep 7, 2026
Merged

lilyshen0722 merged 36 commits into
mainfrom
feat/pr3-activity-contract

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Activity could report attention that its capped list never showed, and links to older messages could search indefinitely or move the target out of view. This change provides the complete scoped queue, restores working context across navigation and refresh, and implements bounded, keyboard-accessible message landing for Activity and existing channel links.

  • Compute global/per-pod counts before pagination; apply pod scope before returning 50-item pages. Choose the composer’s initial destination from the newest accessible global mention and preserve explicit choices and drafts.
  • Apply the Direction C Activity layout with usable phone filters, complete grouped-update pagination, keyboard focus recovery, custom scope/picker controls, retained-page refresh and Retry, and account-isolated Back restoration.
  • Read canonical #message-<id> and legacy ?message=<id> links through one reveal/search path. Loaded sources avoid history requests; older lookup stops after five pages or one failure, with deliberate Retry. Explicit browsing releases the landing protection.

Type

  • Bug fix
  • New feature

Related Issues

Includes the scan from #1580 and removes completed PR3 allowances; #1580 is closed as superseded with its branch retained.

Activity contract and exact-source navigation. Typed handoff attention remains a follow-on before the complete Activity production cutover.

Test Plan

  • Combined frontend: 102 Jest suites / 733 tests, plus 3 script tests at ac678900.
  • TypeScript and production Vite + SEO build at 1353560f; TypeScript and 133 focused Activity/layout/type-floor tests at ac678900.
  • Account isolation checked independently with constant-account and hydration-gate mutations; regressions fail as intended.
  • Persisted queue/count fixture: 2 tests against ephemeral MongoDB; relevant backend blobs unchanged since that verification.
  • All CI passed at ac678900, including real-database service tests (34111582337), Playwright (34111582426), and CodeQL (34111578652). Independent code gate cleared the same head.
  • Independent production-build Activity browser approval at ac678900: 1440, 1200, 900, and touch/mobile 390. History recovery gate at ce685841 carries through unchanged runtime to the final head.

Notes for Reviewer

Opening a source never acknowledges it. Approval/decision ruling paths remain protected; no generic dismissal endpoint or legacy data sweep is introduced.

The earlier history browser pass verifies cold loaded/page-two landing, hash/query links, focus, bounds, failed Retry and repeat quote after Collapse. It used intercepted API fixtures, not production. Failure/bound recovery now lives outside the scrolling transcript; one renderer and its positioning are guarded by tests. Independent history browser gate passes visible recovery, bounded Retry, explicit browsing and exact landing. Final Activity approval verifies the corrected 900px grid, physical phone popup taps, Escape focus with the global handler, counts, and assigned typography cleanup. These browser checks used intercepted API fixtures. Merged as 878f887a872a159d51dde393c7d057287bb48e55; production deployment and live verification wait for typed handoff semantics (TASK-125).

lilyshen0722 and others added 6 commits September 6, 2026 22:54
Ruling (h): mono never below 11px, sans never below 12px, and 11px is
mono-only — so a rule sizing below 12 must name the mono family in the same
rule. Inheriting the family at 11 is banned, which is what keeps this
decidable without a browser.

The four per-component `is 11px` assertions in v2-layout-invariants read like
a floor and are not one. They name specific rules carrying the right value and
say nothing about the wrong value appearing elsewhere, so 87 rules were below
the floors with the suite green.

The allowlist is the burn-down, keyed to the PR that owns each surface, and it
only shrinks: a new violation fails the first test, a fixed one fails the
second. `font-size: var(--v2-fs-*)` is resolved from the sheet's own custom
properties first, or the 15 rules that size from a variable would silently
fall out of scope.

Mutation-checked in both directions: sizing `.v2-shell` to 9px gives 1 red,
and fixing an allowlisted rule to 12px gives 1 red. A rule-count assertion
guards against the parse returning nothing — the first draft of this file
passed `open` to `lastIndexOf`, whose second argument is inclusive, and every
one of the 1,572 selectors came back empty while two of four tests still
passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A container at-rule's body slice ran only to the first inner rule's `}`, so
that rule's declarations were attributed to the `@media (...)` prelude. Every
media block's first child was keyed to its at-rule: 28 such entries, 2
carrying a font declaration. Neither violates today, so the scan's result is
unchanged — but a sub-floor rule written first inside a media block would have
been reported under a selector nobody can allowlist or fix by name.

Container at-rules now open a scope instead of emitting a rule. `@font-face`
stays a rule, because it is a declaration block rather than a container.

Regression test asserts no selector begins with `@` and that
`.v2-verification-banner` — the first child of the 480px block, exactly the
mis-keyed position — parses under its own name. Removing the skip gives 1 red.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR 2a rebuilt the composer and `.v2-root button.v2-composer__send` now sizes
above the sans floor, so the allowlist carried a selector that already
complied and the second test failed on rebase. That is the mechanism working:
the list only shrinks, and a fixed entry must be removed by hand rather than
rotting in place. 87 → 86.

Re-breaking that rule to 11px sans gives 1 red, so removing the entry did not
remove the coverage.

Group headers lose their hard-coded counts in the same commit. A number that
nothing asserts goes stale on the first burn-down step — this one already had,
reading 24 for a group of 23 — and a wrong count in a comment is worse than no
count. The list length is the count.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

CODE GATE: clear at 7632f0e4.

Note on heads: this PR moved twice while I was measuring — aabd6be3441d872a7632f0e4. I had a gate written against the first with a blocker in it; that draft is void and I did not file it, because the blocker was fixed at 441d872a. Everything below is measured at the current sha. The 441d872a → 7632f0e4 delta is V2ActivityPage.tsx and its test only; the service is byte-identical.

composePodId implements ruling 64685

const composeMention = valid.find((row) => row.kind === 'mention') — from valid, before both scope and pagination. valid is sorted priority-then-createdAt-desc, so its first mention is the newest accessible open global mention.

Measured on the case the ruling names — 50 approvals in podA, one mention in podM:

aabd6be3 now
global null podM
scoped to podA null podM

At aabd6be3 it was picked.find(...) over the scoped page, which failed the ruling twice over: the mention sat one past the page, and scoping recomputed it from the filtered set. Both are gone.

Pinned in both directions:

  • composeMention = null (page-derived again) → 1 red
  • computed from the scoped set rather than the global one → 1 red

The second mutation is the one worth having. A fix that only handled the beyond-the-page case would still let a pod filter move the composer's default, and the test catches that separately.

Verified good

  • Caps gone; server slice with limit clamped to 50, offset non-negative, route 400s out-of-range instead of silently clamping.
  • count/countsByPod from valid before the page; remaining/hasMore derived, not counted independently.
  • Scope before pagination, for the reason the comment states.
  • Exact-context landing: openPod(targetPodId, messageId) appends #message-<id>; the label falls back to activity.openPod with no messageId — the source-less decision case.
  • attentionQueue.counts.test.js moves 12/850/46 deliberately, with the second page and the scoped case asserted.

Suites at this head: 18 frontend Activity, 28 across the three backend suites.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addendum — the same-scope refresh race, checked at 7632f0e4. Closed and pinned.

The report is accurate about aabd6be3: loadMoreQueue rejected only a changed scope, so a delayed page could append after the first page had been reset in that same scope. queueGenerationRef did not exist there — zero occurrences at that sha.

At this head it does. The load effect bumps a generation (:127-128) on every run, loadMoreQueue captures it (:199) and rejects on either a scope or a generation mismatch (:209).

Mutating that line back to the aabd6be3 form — scope-only — reddens exactly one test: "discards a late page after a same-scope refresh resets the queue". So the race is not just fixed, it is guarded against re-introduction.

Three details worth recording, since the report names two triggers:

  1. Acknowledgment and refresh are the same race, not two. acknowledgeMention ends in setReloadKey(v => v + 1), which re-runs the load effect, which bumps the generation. The ATTENTION_CHANGED/focus listener does the same thing. One guard covers all three entry points because they share one mechanism.
  2. remaining cannot go stale from a late page. The generation check returns before setQueueRemaining, so a rejected response contributes neither rows nor metadata.
  3. The spinner is guarded separately. finally only clears queueLoadingMore when the generation still matches (:227), so a late response cannot clear the loading state belonging to a newer load. That is the part which would otherwise leave the button enabled mid-flight.

Gate stands: clear at 7632f0e4.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE: CHANGES_REQUESTED @ 7632f0e — Activity still misses the C board, navigation restoration/focus, refresh recovery and the type floor.

Reviewed final head 7632f0e, rebuilt after the generation fix. Production Vite build in an isolated worktree; MCP Playwright at 1440 and actual hasTouch/isMobile 390. Activity/history responses and writes were intercepted. This is PR-build verification, not a live-site smoke or independent backend persistence test.

Required corrections:

  1. frontend/src/v2/v2.css:8629, :8646 and frontend/src/v2/components/V2ActivityPage.tsx:631: the accepted Activity restyle is absent. Desktop header measures 89px with 34px title; phone header 166px with 26px title. Binding 64326(b)/64332 requires the 52px C page bar with 15px/600 sans title and inline subtitle. Desktop inbox is 1120px wide instead of the board's 760px. The page still shows separate “What your agents did” and “Board” sections instead of the pod-grouped “Moved forward” treatment. Both native selects remain; decision choices are pills and Other… is a bordered pill rather than cobalt text. Implement the accepted board/row anatomy, scope controls and PR3 rail/type cleanup. The existing composer remains subject to Sam's new default ruling; this review does not reopen its removal or the settled Settings/Connectors header exception.

  2. frontend/src/v2/components/V2ActivityPage.tsx:236: Back loses working context. Repro at both widths: load 56 through Show more, focus the final row's Open thread, press Enter, then browser Back. Return contains only 50 rows, the page is at the top, and focus is on body rather than the originating row. The compose draft is also lost. Preserve loaded pages/scope/scroll and originating-row focus across this navigation, as the exact-context contract requires.

  3. The same navigation reaches #message-2032, expands the closed thread and all ten replies, and visibly lands on reply 2032 inside the transcript. However, keyboard focus remains on body after Enter activation. Complete destination focus handoff in the Activity/thread navigation path; a route/hash and visual highlight alone do not satisfy keyboard landing. Show more also removes its focused button after the final page and drops focus to body; preserve a useful focus destination on completion.

  4. frontend/src/v2/components/V2ActivityPage.tsx:161 and :450: after 65 rows are loaded, make the next first-page queue refresh fail. All rows disappear and the UI says “Try again” with no Retry control (0 Retry buttons). Retain the existing list on refresh failure and provide an explicit Retry; distinguish initial loading/error from a completed empty list. The separate Show more error/Retry path works.

  5. frontend/src/v2/v2.css:8909: the new Show more control computes to 11px IBM Plex Sans and ink, not the required cobalt control above the sans floor. Running #1580's exact scan from 5739a03 against this stylesheet produces 4/5 pass, with one new violation: .v2-activity__queue-more — 11px without the mono family declared in the rule. Fix the sizing/family and color; do not add an allowance. Existing PR3 burn-down entries also remain untouched.

Passing browser checks at both widths: 0/1/50 boundaries without paging; 56 → 50 + “Show more · 6 remaining” → 56; nine scoped rows/count from a pod whose items follow 56 global rows; global composer candidate honored and explicit target/draft retained across scope change; exact folded source visible/landed with ten replies; source-less decision labeled Open pod and opens without a hash; opening performs no acknowledgement; mocked acknowledgement updates 2→1; Show more failure retains 50 rows and Retry reaches 65; no horizontal overflow.

Backend global-default calculation is now before scope/page in valid; Sprint Review's code gate independently covers its priority/scope guards. I have not repeated their persistence or generation-race proof. Older-history lookup/bound/retry and a separate source-bearing decision browser fixture are not claimed as covered by this walk; complete those on the corrected-head review.

Evidence retained locally: .dev/signal/walk-1588.js, 1588-final-browser-results.txt, 1588-{1440,390}-{first,expanded,landed,scoped}.png, and 1588-sheet.png. Board: https://api.commonly.me/api/uploads/1788694897535-779943629.png . Current full contract: https://api.commonly.me/api/uploads/1788770222316-381737477.md .

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE addendum @7632f0e4 — the existing five-page history/error contract fails in the browser. Overall CHANGES_REQUESTED stands.

Following Sam64710, rebuilt the same head and exercised Activity → source-bearing decision → exact reply2032 at1440 and actual touch390. Final probes use fully mocked auth, pod and message APIs; no live writes or credentials are needed by the retained harness.

  • frontend/src/v2/components/V2Thread.tsx:619–633: an absent source requests seven older pages without a further gesture. The sixth request already exceeds the five-page bound. The harness returns six full50-message pages, then an empty seventh response to stop enumeration; seven is the harness cutoff, not a product limit.
  • frontend/src/v2/hooks/useV2PodDetail.ts:288–323: a failed older fetch is automatically requested again at the identical before cursor. Both widths made four503 requests without pressing Retry, followed by the harness's empty fifth response to terminate the run. loadingOlder resets while hasMore remains true, feeding the landing effect again.
  • After those harness stop responses, the transcript shows its ordinary “beginning of the pod” edge, the unresolved target hash remains, and there is no target-not-reached explanation or Retry control. That edge accurately reflects the artificial empty response; it does not explain the failed link. During lookup the ordinary loading edge is the only progress treatment. Implement visible bounded lookup progress, a stopped error with deliberate Retry, and honest not-reached text with continued browsing, per the existing contract.

The same harness includes a decision source arriving in page2 and an already-loaded folded source. Page2 yields .v2-msg--landed; desktop target is visible. The loaded-fold case currently makes one older request despite already holding the source. Phone samples carry the landed class but fail full transcript-viewport containment after500ms; do not count a class alone as success. These are additional guards for the corrected-head walk, not a replacement for the earlier full passing loaded-history fixture. Review the reveal/scroll anchoring path together with the bound so loaded targets do not fetch and successful landing remains visibly at the destination.

Artifacts: .dev/signal/walk-1588-history.js, 1588-history-final-results.txt, 1588-history-{1440,390}-{missing,error,found,loaded}.png. The final harness dismisses the first-run overlay locally and uses chronological older-page fixtures. Earlier harness runs encountered a cleanup error and then exhausted the smoke account's20-per15-minute login allowance; those tool failures are not product findings. The final run mocks authentication and does not call login.

@samxu01
samxu01 force-pushed the feat/pr3-activity-contract branch from 5b4df33 to e4242fa Compare September 7, 2026 09:05
@samxu01
samxu01 force-pushed the feat/pr3-activity-contract branch from e4242fa to 5a8b493 Compare September 7, 2026 09:05

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

CODE GATE @ 5a8b493ablocked on the markHandled write. (Head moved from 5b4df331 while I measured; the delta is V2ActivityPage.tsx only and markHandled is byte-identical, so everything below is measured at this sha.)

1. No kind filter — the authority boundary from 64721

attentionItemService.ts:423-426:

{ _id: attentionItemId, recipientUserId, status: 'open' }

Any open item the recipient owns resolves — including approval and decision, which have their own ruling paths. POST /:activityId/handled (routes/activity.ts:218) exposes it behind plain auth.

The convention sits eight lines above. acknowledgeMention (:416) selects { _id, recipientUserId, kind: 'mention', status: 'open' }. This is not a missing feature on a new API; it is a sibling written without the constraint its neighbour already carries.

2. resolvedBy: 'handled' is not a valid enum value

models/AttentionItem.ts:55 is enum: ['replied', 'acknowledged'], and the model is not in this PR's diff. Mongoose runs validators on updateOne only with runValidators: true; it is not passed here, and there is no global default — the backend's only two occurrences are explicit call sites in agentMemoryService. So this writes a schema-forbidden value silently, with nothing surfaced.

Downstream, not hypothetical: the TS type is 'replied' | 'acknowledged', so any future switch is non-exhaustive against real rows, and #1577's audit selects on resolvedBy: 'replied' and will not see these at all. Harmless today, wrong the moment per-kind counts are computed from this field. Either add 'handled' to the enum and the union in the same commit, or don't write it.

3. The test cannot distinguish the two implementations

attentionItemService.test.js:206 is titled "marks any open attention item handled by its recipient" and asserts expect.objectContaining({ recipientUserId, status: 'open' }). objectContaining passes with or without a kind term — so it is satisfied by the corrected implementation too. It documents the defect as intended behaviour while being incapable of detecting it in either direction.

Clearing it

Per 64721 the write leaves PR 3. If it returns under TASK-125: an allowed-kinds term in the selector, a rejection test per disallowed kind asserting modifiedCount === 0 for an approval and a decision, and a mutation removing the kind term that turns those red. Enum and TS union move in the same commit.

Not part of the block

The queue work I cleared at 7632f0e4 is unchanged in substance; the same-scope generation guard survives at this head. I have not re-run the full suites here because the block is structural — I will on the next head.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Second blocker at 5a8b493a: the Back snapshot is a cross-account leak on a shared browser. Measured, not inferred.

The chain

  1. Unscoped key. ACTIVITY_SNAPSHOT_KEY = 'v2:activity:snapshot' (:100) — a constant. No user id, no account term.
  2. Hydrated before anything is checked. useRef(readActivitySnapshot()) at :131 runs during the first render — before effects, before auth, before any fetch. readActivitySnapshot (:102-116) validates exactly one thing: savedAt within 10 minutes. It never looks at who saved it.
  3. Logout does not clear it. AuthContext.logout (:143-148) removes localStorage.token and resets React state. It does not touch sessionStorage. Nothing else in frontend/src clears this key except the page's own TTL and restore paths.
  4. The payload is the user's private content. recap, queue (attention titles, actorName, detail, podName), queueCount, queueCountsByPod, and replyDrafts.

So: user A opens a thread from Activity — snapshot written. A logs out. User B logs in in the same tab within 10 minutes. B's first render of Activity hydrates A's queue, A's per-pod counts, and A's unsent draft text.

The drafts are the part that doesn't self-heal

queue and recap are at least overwritten once B's fetch resolves — B sees A's data for a render or two. replyDrafts has no server source: useState(restoredSnapshot?.replyDrafts || {}) at :158 is the only writer besides the textarea. Nothing overwrites it, so A's unsent text sits in B's composer until B navigates away or the tab closes.

What would clear it

  • Key the snapshot by account — v2:activity:snapshot:<userId> — or store the id inside and discard on mismatch. A TTL is not an identity check.
  • Do the identity comparison before the value reaches useState, since :131 and :158 both consume it during first render.
  • Clear the key on logout, so the window does not depend on the TTL at all.
  • Per the operator's note, revalidate retained pages rather than trusting them: a restored queueCount/queueCountsByPod is a number rendered as authoritative that no request produced in this session.

I have not tested whether B's fetch fully replaces every restored field; the draft path alone is sufficient to block, and the identity check makes the question moot.

This is separate from the markHandled block filed earlier at this head.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Gate at b75fd34c — the head moved on from b7bba47b, and two of the three things named there are already resolved here. What remains is one unpinned fix and one contract miss.

Resolved at this head

Lane split is clean. git diff --name-only <base> <head> contains none of V2Thread.tsx, V2MessageRow.tsx, useV2PodDetail.ts — this is the check I said I would run, and it passes. The history files were present at b7bba47b; they are gone now.

The untyped /handled write is gone. Zero occurrences of markHandled in backend/services or backend/routes. Both defects I filed with it — the missing kind filter and the out-of-enum resolvedBy: 'handled' — go with it.

Account isolation is real. The key is now ${ACTIVITY_SNAPSHOT_KEY}:${accountId} (:105), readActivitySnapshot requires an id (:107), and hydration moved out of first-render useState into an effect gated on authLoading and accountId (:180-182), with the effects keyed on accountId. Drafts are restored via setReplyDrafts(snapshot.replyDrafts) inside that gate (:192), so preservation survives while the leak does not — which is the shape both the ruling and the blocker needed.

Unpinned: nothing tests the isolation

Replacing the key function with the old unscoped constant:

const snapshotKey = (_accountId: string) => ACTIVITY_SNAPSHOT_KEY;

leaves 19/19 green. No test in V2ActivityPage.test.tsx names a snapshot or an account, and the suite's list confirms it — every test is about queue paging, scope, landing or retry.

So the security fix is exactly one line from being undone with nothing to notice. Two cases pin it: same account, Back restores queue and draft; different accountId in the same tab, snapshot discarded. The second is the one that fails today against the mutation above.

Still open from the operator's list

The native <select> survives at :603<select value={composePodId}> over recap.pods — against the no-select contract. That one is real and unaddressed at this head.

I have not re-checked the Back-revalidation extent (56 rows hydrated, {limit:50, offset:0} requested, unconditional setQueue) because it is being corrected; I will measure it on the next head rather than describe it from the previous one.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#1588 UX-GATE: CHANGES_REQUESTED — combined candidate d1a6b76

Reviewed exact combined candidate d1a6b76c6d14ffa14e89d65af27fd21170ef5ddf (codex/decision-integration), not an assertion about the separate PR branch head. Fresh production Vite build; MCP Playwright 1440 and actual touch/mobile 390. All API/auth/write fixtures intercepted; no live login, production write, merge or deploy. Code gate is separate.

Passing at both widths: 0/1/50 queue boundaries, 56→50+6, nine-item scope beyond the first global page, global initial compose destination and intentional destination/draft across scope changes; no native select or horizontal overflow. Loaded short-text source 2032 opens all ten folded replies, lands visibly and receives keyboard focus. Source-less decision opens only the pod without acknowledging. Final Show more transfers focus to first appended row. Back restores all 56 rows after settled revalidation, draft and origin-row focus; scoped Back restores nine rows and selected scope. Existing mention acknowledgment fixture updates 2→1. Desktop title is 15/600 sans, header 52px, queue 760px; Needs you then Moved forward is present.

Blocking history findings (both widths unless specified):

  • Cold Activity source-bearing decision → source on older page two stops at Target not found after 0 older pages with one incidental older request; target never arrives. Loaded and legacy ?message=2032 variants receive the target focus/landed class but retain that same erroneous not-found status. An unnecessary older fetch then leaves the target outside the messages scroller. Screenshot confirms the viewport shows earlier rows, not the focused target. The shorter Activity fixture passes; the long-row history fixture exposes the interference.
  • On a settled pod, an absent target does stop after five lookup requests and presents the honest bound plus Retry. Do not read the fixture's total seven requests as seven requests before a gesture: five occur before deliberate Retry, two afterward. The five-page counter itself works in that case.
  • Error recovery still admits automatic requests outside the stopped search. In the settled error case one 503 stops correctly; one deliberate Retry is then followed by four requests (three more 503s and an empty harness cutoff) without another gesture. In cold missing, one incidental request precedes Retry, then six requests follow that one Retry. Check the ordinary IntersectionObserver/loadOlder path alongside the bounded search; keep it from racing an exact-target lookup or restarting after failure/bound without deliberate browsing.
  • Settled source-on-page-two lookup focuses and lands; desktop containment passes, 390 containment fails. Cold navigation must work independently of warming the pod first. Canonical hash and legacy query parsing both exist; parsing alone is not a landing pass.

Blocking Activity findings:

  • Failed next-page Retry refreshes offset 0 instead of retrying the failed append: 50 rows remain, requiring another Show more. A failed first-page refresh retains 65 rows and exposes Retry (improved), but successful Retry replaces them with 50 and drops focus to body. Preserve loaded extent through refresh/retry, not only Back. Fresh first-page queue failure has no Retry button at all and renders the same 0 description despite unknown count.
  • Composer picker option activated with Enter closes the menu and leaves focus on body. Restore focus to its trigger and finish the keyboard interaction.
  • Visual restyle remains incomplete: mobile header is 149px rather than the 52px page bar; desktop header is inset at x195/y43 rather than the panel's x85/y15 bar. Decision options still have 999px pill radii (30px desktop height /44 touch); Other… is an ink dashed pill instead of cobalt text. Show more computes 12px sans but ink instead of cobalt. Day selector remains sans; board uses mono. Rail language caret remains. The retained composer still carries Posts as you copy; retaining the composer does not restore the removed posts-as chrome.
  • Exact #1580 scan from 5739a03 against this tree is 4/5: the old .v2-activity__queue-more sizing rule still resolves to 11px without mono even though a later override computes 12px in browser. Remove the violating rule and complete PR3 burn-down; do not add allowances.
  • Moved forward fixture with 25 updates shows group count 25, 17 more, then only 20 rows. Five updates are silently unreachable/unexplained. If keeping the 20-row cap, disclose the omitted remainder and its destination; the current group count/More combination is misleading.

TASK-125 typed handoff semantics remain after #1588 per Sam64721/64770; absence of a generic handled endpoint is correct. Account-isolation mutation proof belongs to Sprint Review's independent gate; not claimed by this browser walk. Repeated quote-after-Collapse and real persisted acknowledgment were not rerun in this candidate pass; no live smoke claimed.

Reproduction artifacts: .dev/signal/walk-1588c.js, walk-1588c-extra.js, walk-1588c-history.js, and diagnostic walk-1588c-warm-history.js; matching 1588c-*-results.txt, screenshots and 1588c-sheet.png. Warm fixture resets its request counter after initial navigation and can return a duplicate cursor page; do not infer a product duplicate-fetch defect from that diagnostic alone. Cold fixture and its screenshots are the source for initial-navigation failures. Initial harness strict-selector timeout (two count elements) was fixed; it is not a product issue.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE: CHANGES_REQUESTED @ 290707c — Activity refresh/append Retry, picker focus and C visual corrections remain.

Reviewed exact #1588 head 290707c. Production build, fully mocked auth/API/writes, MCP Playwright at 1440 and actual touch 390. This is the Activity branch, not the combined integration candidate: Kai's history implementation and the integration-only test delta are not included here. No live smoke or real writes.

Confirmed remaining at both widths:

  • Load 65 rows, fail a first-page refresh: all 65 remain and Retry appears. Successful Retry still replaces them with 50, removes Retry and leaves body focus. Draft survives. Preserve the loaded extent through successful refresh and its retry, including focus when the initiating control disappears.
  • Fail Show more at offset 50: 50 rows remain with Retry. Retry requests offset 0 and leaves 50 rows; it does not retry the failed append. A second Show more is required.
  • Composer destination option selected with Enter closes the picker but leaves body focus. Restore trigger focus.
  • Fresh queue failure now exposes one Retry button (fixed), but still renders “the same 0 as the rail and the inspector.” The failed read does not establish zero.
  • Moved forward fixture has 25 updates: header says 25, starts with 3 lines and “17 more”, expands to 20, with five unexplained and unreachable.

Visual misses against the accepted Activity board remain: mobile header is 149px, not 52px; desktop header sits x195/y43 rather than at the full panel edge x85/y15. Decision options have 999px radii, 30px desktop / 44px touch heights; Other… is an ink dashed pill. Show more computes 12px sans but ink. Day selector is sans rather than mono. Rail language caret and composer “Posts as you” copy remain. Composer itself must remain per ruling 64685.

Passes: 0/1/50 boundaries; 56→50+6; global 65→scoped nine; initial global composer destination and intentional choice/draft across scope; Back restores 56 rows, draft and origin row focus after settled revalidation; scoped Back restores nine and scope; final Show more focuses the first appended row. Source-less Open pod produces no hash or acknowledgement. Mocked mention acknowledgement 2→1. No horizontal overflow. Desktop title 15px/600 sans and queue 760px.

The old 11px sans queue-more rule is corrected: the exact type-floor scan from #1580 / 5739a03 passes 5/5 against this CSS. This clears the new-selector violation; it does not remove the outstanding PR3 burn-down entries. The short loaded-fold source still opens all ten replies visibly with the unchanged #message-2032 producer; destination focus is body in this unintegrated branch. Keep Kai's history/landing lane and integration gates separate; this review does not supersede the combined history findings.

Evidence: .dev/signal/walk-1588d.js, walk-1588d-extra.js, 1588d-browser-results.txt, 1588d-extra-results.txt, 1588d-sheet.png and 1440/390 screenshots. Initial build invocation used the wrong cwd and was corrected; initial test command assumed react-scripts and was replaced with this repository's Jest binary. Both were harness setup errors, not product failures. Final build and browser walks completed.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

HISTORY UX-GATE: CHANGES_REQUESTED @ 702752b — lookup and landing behavior pass, but stopped-search status and Retry are offscreen after cold failure/bound at 1440 and actual touch 390. Activity UX remains CHANGES_REQUESTED separately.

Built exact integration 702752b in an isolated worktree. Fully mocked authentication, pods, history, Activity APIs and writes; no real login, production mutation or live smoke. Canonical links are produced by keyboard activation of a source-bearing Activity decision; legacy links navigate with ?message=2032.

Remaining blocker: after a cold missing/error link settles, the stopped-search status and Retry are mounted above the visible scroller. The reader sees ordinary messages with no visible failure/bound explanation. Before any harness scroll or Retry click, measured status top/bottom: desktop missing -2143/-2115, desktop 503 -709/-681 (scroller starts at 66); mobile missing and 503 -1860/-1816 (scroller starts at 51). role=status existing in the DOM does not make it visible. Keep search progress/failure/bound recovery visible within the transcript viewport when landing fails, while preserving the working request limits and deliberate browsing. This was exposed during screenshot inspection, then independently measured in four cold status fixtures.

The following functional checks pass at both widths and both link forms:

  • Cold target on older page two: exactly two older requests, source 2032 revealed inside its folded thread, visible inside the message scroller, landed and holding actual DOM focus after settling. No stale zero-page status.
  • Cold loaded folded source: zero older requests; all ten replies reveal and target stays visible/focused. Separate flat-target fixtures also pass with zero requests after 1.2 seconds.
  • Missing source: exactly five successful older requests, then an honest bounded-search status and Retry. Scrolling the scroller to the edge does not fetch a sixth. Deliberate Retry permits exactly five more and stops again; a deliberate Load earlier gesture then fetches one page and stops. The totals of ten/eleven include those explicit gestures.
  • Persistent 503: one request, error and Retry. Scrolling to the edge leaves the count at one. One deliberate failed Retry makes exactly one more request and stops again. No sentinel bypass.
  • Repeat quote after Collapse: first and second activation both reveal all ten replies and visibly land/focus source 2032 with the same canonical hash. The ordinary pod mount made one older request before either gesture; neither quote added a request. This is distinct from the cold target-link zero-fetch cases.

The missing/error fixtures no longer synthesize an empty response to stop loops: a safety abort above twelve requests was never reached. The failed-history fixture stays failed, so no false exhaustion can be inferred from fixture behavior. Initial short-SHA local fetch failed and was corrected to the branch ref before the exact detached checkout; no product failure.

During review the integration branch advanced to 242c9ba. Verified its only delta from 702752b is 31 lines in V2ThreadReveal.test.tsx; all application blobs are identical. Carry this history CHANGES_REQUESTED verdict to 242c9ba. The added test itself remains the code reviewer's gate. No blanket #1588 approval, Activity restyle approval, account-isolation mutation claim or live-site claim is made here.

Evidence: .dev/signal/walk-1588h-history.js, walk-1588h-flat.js, walk-1588h-repeat.js; corresponding 1588h-*-results.txt; 1588h-sheet.png and individual 1440/390 screenshots. Closes the prior cold readiness, landing visibility/focus, loaded-fetch and sentinel-bypass findings; the hidden stopped-search recovery state remains. Additional evidence: walk-1588h-status.js and 1588h-status-results.txt.

@lilyshen0722 lilyshen0722 changed the title feat: expose complete Activity attention queue Complete Activity attention queue and exact-source navigation Sep 7, 2026

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE: CHANGES_REQUESTED @ d5cda83 — Activity browser gate; history recovery visibility remains a separate hold.

Built and walked 1353560, then rebuilt d5cda83 and repeated the main Activity and Retry walks plus the new failed-scope/Back case. Actual 1440 and 390 touch contexts, production Vite builds, fully mocked authentication/API/writes. CSS is identical across those two heads. No live-site or persistence claim.

Required corrections

  1. Pane bar still inset; mobile controls hidden. frontend/src/v2/v2.css:5082 leaves .v2-feature__body at padding: 28px 34px 42px; :5089 caps the legacy wrapper at 1240px. The Activity bar measures x135/y43/w1240 instead of the board's pane x85/y15/w1340. On 390 it starts x34/y28/w322. Changing Activity's own width did not remove its ancestors' inset. The new mobile rule at :11498 makes the entire header horizontally scroll: 677px content inside 322px. At rest even Today is clipped and all scope controls are offscreen; selecting Review Nine scrolls the header by 355px and moves the Activity title to x-309. Keep the 52px bar/title and scope usable together inside the pane.
  2. Control cascade and type floor remain unfinished. Show more computes ink rgb(16,24,40), despite the cobalt declaration at v2.css:11484. Decision options compute 30px desktop (32px contract), 44px touch; radius is now correctly 4px. Day is now correctly Plex Mono, but pod-scope controls remain sans rather than the board's mono segments. Tell your agents computes 11px Plex Sans (:8762), and desktop EN computes 11px Plex Sans (:602). The PR3 burn-down still needs correction, not new allowances.
  3. Moved forward stops at 20 of 25. frontend/src/v2/components/V2ActivityPage.tsx:936 still slices to 20: three rows + 17 more expands to 20, followed by the new truthful “5 more updates in Review Alpha not shown” note. There is no further load action or destination (only Show less). Walk 2 §3 explicitly requires groups over 20 to paginate by 20. The explanatory note improves the old unexplained discrepancy but does not make the remaining five reachable.
  4. Finish the board's row and empty-state anatomy. Desktop actions still sit below the copy instead of at its right; the decision mark is ink instead of cobalt (see board comparison). A successful zero queue still shows a bordered, centred Nothing open card with secondary coaching copy; walk 2 §3 specifies one muted mono line without a frame. The legacy footer sentence also remains (walk 2 miss 39). These are existing board misses surfaced by the full pass, not new regressions introduced by the scope fix. The restored composer itself remains required by ruling 64685.

Verified corrections and behavior

  • 0/1/50 row boundaries have no Show more; 56 appends 50+6; global 65 scopes to the nine rows beyond the first page, count nine. No document horizontal overflow (the mobile header has its own problematic overflow described above).
  • Failed append retains 50 and Retry; Retry requests offset 50 and reaches 65. Failed same-scope refresh retains 65 and Retry; successful Retry retains all 65, draft, and focused row ux50. Final append focuses the first appended row.
  • Picker Enter returns focus to its trigger after animation-frame settling (checked at 250ms). An immediate probe saw body before the scheduled focus; that was probe timing, not a remaining picker defect.
  • Main Back restores 56, draft and origin row ux55 after revalidation; scoped Back restores nine, selected scope, draft and origin ux108.
  • New d5cda83 failure probe: load global 65, switch to Review Nine with its fetch failing → zero old rows, Count unavailable and Retry, draft retained. Retry → nine/count nine; open and Back → nine/draft/origin focus. The probe's broad count-selector count includes the separate Moved forward badge; it is not a Needs you badge. Recap mock is deliberately static and is not evidence about recap scope filtering.
  • Initial compose destination consumes the global API candidate; intentional accessible choice/draft survive scope and refresh. Backend candidate derivation/authority and account isolation are separately reviewed by peers, not independently proved by these browser mocks.
  • Source-less Open pod adds no hash or acknowledgement. Loaded folded exact source 2032 becomes visible, landed and focused with all ten replies. Mocked mention acknowledgement changes 2→1; no real write.
  • Fresh queue failure now reports Count unavailable with Retry, rather than asserting zero. Other is cobalt text with no border/fill. Rail caret and Posts as hint are removed; hint now only gives the send shortcut.

Exact #1580 scan from 5739a03 against this identical CSS: 4/5; no new below-floor selectors, with the only failed test caused by the now-stale .v2-lang-switch__caret allowance after its legitimate deletion. Remove that stale allowance when integrating #1580. This is distinct from the remaining existing PR3 floor entries; do not report a clean floor merely because there are no new violations.

History hold is unchanged: the previous cold missing/error walk found status/Retry outside the visible transcript viewport. This Activity gate does not rerun or clear Kai's pending visible recovery correction.

Evidence: .dev/signal/walk-1588e{,-extra,-style,-scope,-d5,-d5-extra}.js, 1588e-{browser,extra,style,scope,d5-main,d5-extra}-results.txt, 1588e-sheet.png, individual screenshots. Local worktree setup initially used FETCH_HEAD from the wrong worktree; corrected to the full d5 SHA before the final build and runs. No implementation, merge, deployment, real login, or live API write.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE: CHANGES_REQUESTED @ e7b8085 — Activity popup stacking/Escape focus and the 900px pane remain blocked; history recovery UX is APPROVED.

Reviewed production builds at 1440 and true-touch 390, plus the requested 900/1200 responsive checks. Auth, API responses and writes were fully mocked. Final head: e7b8085. Activity was fully walked on c37e54f, then rebuilt e7b8085 and rechecked its EN change, mobile tap failure and 900px geometry. Intervening 3aeaaf0 changes only two test lines; e7 changes only EN CSS and its invariant. No merge, deployment or live-account writes.

Remaining Activity corrections:

  1. 390px overflow menu is behind the composer destination control. With twelve pods, open More and scroll its menu to the last choice. The centre of “Overflow pod 5” is covered by .v2-activity__compose-picker-button. A real touchscreen tap there opens the composer picker, leaving the scope menu open and the scope unchanged. Last-choice selection passes, but does not prove the middle choices are unobstructed. Playwright locator.click can find an exposed edge and succeeds; the centre-point hit test and physical tap establish the defect. Put the popup above the entire composer, not just its background.
  2. Escape loses focus. Open the scope menu, focus one of its choice buttons, press Escape: menu closes, but document.activeElement is BODY after 300ms. Reproduced at 1440/1200/900/390. Clicking a choice correctly returns focus to the trigger and preserves its selected name; Escape must do the same in the real page, including any outer keyboard handling.
  3. 900px Activity uses only a 258px pane. The pane content is x85..343 despite a 900px viewport; most of the screen is blank. Decision copy becomes one character per line. The 300px scope popup starts at x31 and its labels are clipped by the pane edge at x85. This is independently rendered evidence from the requested intermediate-width check. The title and controls are siblings; this is not the withdrawn fixed-header-child explanation. Check the feature-wide shell grid cascade at the intermediate breakpoint.
  4. The assigned PR3 type-floor burn-down is still not complete. EN now computes as 11px IBM Plex Mono and passes; composer label also passes. The exact #1580 scan from 5739a03, copied only into the review worktree, is 4/5: no new violations, but two legitimate stale allowances now need removal when #1580 is integrated (.v2-lang-switch__caret and .v2-rail__utility .v2-lang-switch__trigger). Old assigned rules such as the inspector detail/section headings and tab counts remain below floor. A no-new-violations scan does not clear the existing PR3 cleanup requirement.

Verified Activity improvements: full-edge 52px header (1440 x85/y15/w1340; 390 x0/y0/w390), stable visible title, All pods reachable again, desktop decision and mention actions right, cobalt decision mark/Other/Show more, options 32px desktop and 44px touch with 4px radius, flat mono “Nothing open.”, footer and rail caret removed. Composer remains under the settled ruling. Moved forward 45 updates reaches 3→20→40→45, then Show less→3; final More focuses Show less and collapse focuses 17 more. Overflow last item is visible and selectable, selected name persists, and All pods returns all 50 loaded rows at each tested width.

Functional rerun on c37 at 1440/true390: 0/1/50 boundaries; 56→50+6; 65 global→nine scoped; initial API-global composer target and deliberate choice/draft; failed append retains 50 and retries offset50→65; failed refresh retains65 and successful Retry preserves65/draft/focus; picker Enter focus after settling; Back restores56/draft/origin row; scoped Back restoresnine/draft. Fresh failure says Count unavailable with Retry. Source-less open has no hash/ack; source-bearing open reveals all ten replies and lands visibly with focus. Mock acknowledgement 2→1. Earlier d5 failed-new-scope test carries across unchanged request/snapshot logic. Backend derivation and authority/isolation are peer code evidence, not a browser claim.

History-only UX APPROVED at ce68584, carried to e7b8085 across identical history runtime. Both canonical Activity keyboard links and legacy ?message=2032 at both widths: progress visible; missing stops at5, edge scroll adds0, Retry adds5 then stops, explicit load earlier adds1; persistent503 stops at1, edge scroll adds0, failed Retry adds1, explicit load earlier adds1. Recovery is visibly inside the transcript viewport before any harness scrolling: desktop y74..104; mobile y59..105. Page-two source takes exactly2 older fetches and lands visibly/focused; loaded folded and separate flat targets take0 older fetches, with no stale status. Repeat quote after Collapse opens all10 and lands/focuses again; its ordinary no-hash mount had1 incidental fetch before either gesture, neither quote added a fetch. No safety abort or artificial empty cutoff was reached in bounded-search cases.

Evidence: .dev/signal/1588f-{history,flat,repeat}-results.txt, 1588g-{main,extra,style,overflow-final,tap,900}-results.txt, final 1588g-e7-*.txt, matching walk scripts and screenshots, and 1588g-sheet.png. The initial extra harness used a nonexistent empty-state selector; corrected to .v2-activity__empty--plain, closed its context and reran both widths. That timeout was a harness issue.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE: CHANGES_REQUESTED @175a182d — covered phone tap, Escape focus and rail font now PASS; 900px pane and assigned PR3 type-floor cleanup remain.

Built exact 175a182. Rechecked 1440/1200/900/true390 scope popup and Escape, plus a physical touchscreen tap at the formerly covered middle choice and final rail font. The phone tap now selects Overflow pod 5, closes the scope menu, leaves the composer picker closed and focuses the scope trigger. Escape now returns focus to the trigger at every tested width, including after 300ms. EN is 11px IBM Plex Mono. These supersede those three findings in the e7b8085 review.

Remaining: at 900px the main pane is still x85..343, width258, most of the viewport blank. Decision copy is crushed into one-character lines; the 300px scope popup starts at x31 and is clipped at the pane's x85 edge. The assigned PR3 below-floor declarations also remain, as documented in the e7b8085 gate. No new floor violation is claimed; the prior exact #1580 scan's two stale allowances must be removed on its integration.

The completed c37e54f Activity counts/scope/composer/Retry/Back, 3→20→40→45 Moved forward pagination/focus, full-edge 52px header, right-side actions, flat empty state and controls evidence carries across the inspected bounded delta. History-only UX APPROVED at ce68584 carries unchanged: visible recovery, bounded Retry/explicit-edge behavior, loaded/found landing and repeated quote all passed at 1440/true390. See the previous full gate for precise counts and limits.

Evidence: .dev/signal/1588j-{tap,overflow,900,style}-results.txt, matching walk scripts and screenshots; 1588j-sheet.png. All auth/API/writes mocked. No source implementation, merge, deployment or live-account writes.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE: APPROVED @ ac67890 — the remaining tablet width and assigned PR3 type-floor corrections pass.

Built exact ac67890 and walked the production build with fully mocked auth, APIs and writes at 1440, 1200, 900 and actual touch/mobile 390. This supersedes my 175a182 CHANGES_REQUESTED gate. No live-site smoke, real login or live write was performed.

  • At 900, .v2-pane--main is x84/w802 and Activity is x85/w800 (previously w258). The 760px queue fits; the 300px overflow popup is wholly inside the pane, its last choice is visible and selectable, and decision text reads normally. No horizontal overflow. This validates the feature-wide grid exemption in the actual browser.
  • Repeated popup selection and Escape at 1440/1200/900/390; selection and Escape return focus to the trigger. Physical center touch on Overflow pod 5 at 390 selects that scope, closes the scope popup, and leaves the composer picker closed. Existing popup fixes remain intact.
  • Activity count and composer label compute 11px IBM Plex Mono on both widths; EN computes 11px Mono. Workspace inspector avatar initials compute 12px Sans in their 28px squares at 1440 and in the real phone inspector drawer. Inspector headings compute 11px Mono; sidebar count is 11px Mono; phone navigation displays Needs you · 56 at 11px Mono. Text fits without horizontal overflow.
  • The legacy .v2-mobile-tabs__badge and old .v2-inspector__* selectors have no current TSX render sites; their corrected declarations are scan-covered, not claimed as mounted browser elements. The current phone count is inline in .v2-mobile-tabs__label; the current inspector uses .v2-workspace-inspector__*.
  • Ran the actual integrated type-floor test: 5/5 pass. Compared against published #1580 at 5739a03: only the 20 PR3 allowances and deleted caret allowance leave; parser and five tests unchanged, no allowance additions. 65 unrelated allowances remain; this is completion of the assigned PR3 floor work, not a claim that the entire stylesheet is debt-free.

175a182..ac67890 changes only CSS and scan/invariant tests. Activity request/snapshot/composer behavior and history runtime are unchanged. Carry the completed c37 full Activity functional walk (0/1/50, 56→50+6, global65→scope9, retry/refresh preservation, Back/draft/origin focus, source-less and exact-source navigation, mocked acknowledgment, Moved45→3/20/40/45 and final focus). Carry HISTORY UX APPROVED from ce68584: visible progress/bound/error Retry, five-page bound, explicit edge, found/loaded/repeated-quote visibility and keyboard focus. Backend derivation/authority/isolation are separate peer evidence.

Evidence: .dev/signal/walk-1588k-{900,tap,overflow,style,typography}.js, corresponding 1588k-*-results.txt, screenshots and 1588k-sheet.png. All captures used intercepted data; the sheet includes the accepted Activity board for reference. No implementation, merge or deploy by UX.

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