Skip to content

polish(projects): streamline overview navigation and state - #6600

Merged
thomaspblock merged 6 commits into
projects-channel-first-pt5-navigation-requestsfrom
projects-channel-first-pt6-overview-polish
Aug 24, 2026
Merged

polish(projects): streamline overview navigation and state#6600
thomaspblock merged 6 commits into
projects-channel-first-pt5-navigation-requestsfrom
projects-channel-first-pt6-overview-polish

Conversation

@thomaspblock

Copy link
Copy Markdown
Contributor

Summary

  • make the Projects overview follow the active section with consistent context, empty states, and row alignment
  • add an animated section-local search that preserves sort and responsive tab behavior
  • simplify sidebar and list controls while keeping large project collections incrementally mounted

This is Part 6 of the channel-first Projects stack, based on #6597. The final part contains project workspace aggregation and the default project-home template.

Testing

  • focused search, activity digest, agent selection, related-channel, and overview-context tests: 26/26 passed
  • Desktop unit suite: 5,451/5,451 passed
  • E2E-mode Desktop build passed
  • TypeScript, Biome, and differential file-size checks passed
  • full pre-push gate passed

Post-Deploy Monitoring & Validation

  • exercise every Projects overview section, search transition, selection state, and right context rail in staging Desktop
  • healthy signals: stable tabs, aligned rows, correct section context, and responsive large-list interaction
  • failure signals: search flicker, stale context, wrapped toolbar rows, missing empty actions, or input lag; mitigate by reverting this PR

@thomaspblock
thomaspblock requested a review from a team as a code owner August 23, 2026 03:14
@thomaspblock
thomaspblock marked this pull request as draft August 23, 2026 03:54
@thomaspblock

Copy link
Copy Markdown
Contributor Author

Cassandra adversarial re-review — exact head bff5267d781f0bfaa0f06fc46f36d06d93b4c0b5

Verdict: needs work. I found one security-relevant default-recipient confusion issue. This comment is the durable sink for the finding and current validation residuals.

P1 — A relay agent can become the default recipient by naming itself “Fizz” or “Kit” (confidence 75)

Evidence: desktop/src/features/projects/lib/projectAgentSelection.ts:8-12:

agents.find((agent) => agent.personaId === WELCOME_GUIDE_PERSONA_ID) ??
agents.find((agent) => {
  const name = agent.name.trim().toLocaleLowerCase();
  return name === "fizz" || name === "kit";
}) ??

The full path includes unmanaged relay agents: ProjectsAgentPromptPage.tsx:186-194 appends every eligible relay agent with its relay-controlled name and no personaId; ProjectsAgentPromptPage.tsx:471-474 uses pickDefaultProjectsAgent(candidates) as the selected recipient; ProjectsAgentPromptPage.tsx:534-540 then sends the prompt and repository context to that pubkey. ProjectAgentChatPanel.tsx:106-107 uses the same defaulting helper for detail context.

Trigger: the user has no managed builtin:fizz instance, but a mention-eligible relay agent advertises the display name Fizz (or legacy Kit). Even if a known active managed agent sorts first, the name match overrides it. A normal send then directs the user's prompt plus project/repository context to the name-spoofing agent unless the user notices and changes the picker.

Suggested fix: trust personaId === "builtin:fizz" only for the preferred identity. If legacy compatibility is required, limit name fallback to a locally managed/otherwise cryptographically pinned candidate; never use an unmanaged relay-controlled display name to choose a default recipient. Add a regression test with an active managed first candidate and an unmanaged name-only Fizz candidate.

Coverage

Read the full 47-file 93814111...bff5267d diff and surrounding call paths for candidate eligibility/default selection and send, relay/signer-scoped conversation persistence, project/repository/channel navigation, selection reset/drawer restoration, mutation trigger permissions, local search, activity digest, related-channel collapsing, panel states, and sidebar restructuring. No mutation authorization or cross-relay persistence regression found in the reviewed paths.

Validation / residuals

  • GitHub currently reports head bff5267d, base 93814111, mergeable true, state unstable.
  • CI run 32614868719 is red: Smoke shards 1, 2, and 4 failed; shard 3 was cancelled; aggregate Desktop failed. Desktop Core, macOS build, relay E2E, and both integration shards passed.
  • Focused pure tests for agent selection, related channels, activity digest, and search passed (17 tests). A separate retained-view test could not start in my detached worktree because dependencies were not installed there (jsdom resolution); CI Desktop Core is the authoritative broader unit/typecheck evidence.
  • Exact-head green CI is still required after the finding is fixed. Historical broad smoke timeout cascades are not sufficient evidence to waive that gate.

@thomaspblock
thomaspblock force-pushed the projects-channel-first-pt5-navigation-requests branch from 9381411 to 07d2ce6 Compare August 23, 2026 22:45
@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge review — PR #6600 at head bff5267d781f0bfaa0f06fc46f36d06d93b4c0b5 (draft)

Reviewed against stack base projects-channel-first-pt5-navigation-requests (93814111e). Personas: correctness (always-on), testing, reliability. Verdict: not merge-ready — CI red at head with two deterministic pt6-caused spec breaks. Code substance is otherwise in good shape; findings below.

Blocking

F1 — Intro action copy changed; channels.spec.ts still asserts the old strings. Confidence: 100.
desktop/src/features/channels/ui/useChannelIntro.tsx now emits:

description: "Add an agent here.",
label: "Add agent",

but desktop/tests/e2e/channels.spec.ts (not touched by this PR) still asserts, in empty channel shows intro actions (line 1737):

addAgentsAction.getByText("Add agents", { exact: true })   // :1752
addAgentsAction.getByText("Bring them in.", { exact: true }) // :1755-1757

CI confirms: run 32614868719 Desktop Smoke E2E (1) → ✘ 192-194 channels.spec.ts:1737 › empty channel shows intro actions (failed + both retries). Smoke (1) is green at the base head 93814111e, so this is pt6-caused, deterministic. Fix: update the spec (or revert the copy).

F2 — SidebarProjectsSection is no longer rendered anywhere, but three e2e specs still drive its testids. Confidence: 100.
desktop/src/features/sidebar/ui/AppSidebarPinnedHeader.tsx removes <SidebarProjectsSection /> (old line 186) and its import. At this head, git grep SidebarProjectsSection -- desktop/src matches only the definition file — no renderer remains, so sidebar-projects-section-label, sidebar-projects-create, and sidebar-project-* testids never mount. Still consumed by:

  • desktop/tests/e2e/project-commit-detail.spec.ts:25-26 (addProjectToSidebar helper; call sites 570, 593, 1181, 1239 — and project sidebar rows open the home channel and nest extra channels at ~1394 depends on the section wholesale)
  • desktop/tests/e2e/project-pr-review.spec.ts:71-72 (call sites 1157, 2316, 3161, 3258)
  • desktop/tests/e2e/projects-v3-screenshots.spec.ts:137-138 (sidebar project add flow browses before creating, line 133)

CI confirms the reachable slice: Smoke (4) → ✘ 3-5 projects-v3-screenshots.spec.ts:133 ×3 with locator.hover: Test timeout of 30000ms exceeded — exactly the hover on the never-rendered label. Smoke (3) was cancelled in this run, so the commit-detail/pr-review blast radius is unmeasured, not absent.
Product question for the author: is removing the pinned-projects sidebar section intended in pt6? If yes, the specs (including this PR's own edits to project-commit-detail.spec.ts, which keep addProjectToSidebar) need reworking; if no, the render was dropped accidentally.

Gate

F3 — CI at head is red overall; part inherited from the base. Confidence: 100.
Run 32614868719: Smoke (1)(2)(4) failure, Smoke (3) cancelled, Desktop aggregate failure. The base head run 32611706148 (at 93814111e) fails the same tests for: projects-v3-screenshots restricted-repos / Buzz-loader / workspace-states, the workflows.spec.ts cluster, workflow-local-controls.spec.ts:85, entity-link-recipient-cards.spec.ts:508 — those are inherited stack instability, not pt6 regressions. But inherited or not, the merge gate stays fresh full-green CI at the exact pt6 head after F1/F2 fixes and base stabilization.

Medium

F4 — compatibilityWarning is silently dropped on project create. Confidence: 75.
ProjectsView.tsx replaces the warning branch with unconditional success:

toast.success(`Project "${result.project.name}" created.`);
await goProject(result.project.id);

createProject.ts:41 still returns compatibilityWarning?: string. The only remaining surfacer is SidebarProjectsSection.tsx:373-375 — which is unrendered at this head (F2). Compounding: ProjectsView.tsx now filters the list to projectReadModels.filter(isExplicitProject) (isExplicitProject = !project.legacy, projectModels.ts:53-55), so a compatibility-path creation gets a success toast, navigates via goProject, and the project is invisible in the Projects list. Which test would fail if this is wrong: none — no test covers the compatibilityWarning path at this head.

Minor

F5 — Activity digest week-window freezes between data refreshes. Confidence: 75.
ProjectsView.tsx computes nowSeconds: Math.floor(Date.now() / 1_000) inside a React.useMemo whose deps are only query data. The adjacent feed deliberately ticks (ProjectsActivityFeed.tsx: "Week buckets are clock-derived; ticked so the memo cannot freeze 'This week' across a week boundary" — useNow(600_000)). The digest has the same hazard and no tick. Cosmetic staleness only.

F6 — Orphaned scope machinery + stale localStorage keys. Confidence: 75 (quoted), severity minor.
readStoredRepositoryScope/writeStored*Scope (projectsViewHelpers.ts:111-184) and ProjectsListScopeDropdown.tsx now have zero callers at this head (verified by grep across desktop/src at bff5267d7). Persisted scope values are silently ignored. Residual cleanup, fine to defer or fold into a later part.

Verified clean (scoped to what I actually checked)

  • New lib modules ship with real unit tests: projectsSearch.test.mjs (token matching incl. empty query), projectsActivityDigest.test.mjs (both prefix branches), projectAgentSelection.test.mjs (Fizz persona preference + Kit fallback), projectRelatedChannels.test.mjs (collapse of shared-channel repos). Desktop Core is green at this exact head.
  • useProjectSelection edge-trigger rewrite is correct: wasActiveRef gives one onSelect per inactive→active edge and one onClear per active→inactive edge, including the resetKey render-time reset; the search input's Escape calls event.preventDefault() and the selection listener checks !event.defaultPrevented, so closing search does not clear selection.
  • Drawer save/restore ref (selectionDrawerStateRef) is set once per selection episode and consumed on clear; new e2e selection restores a previously collapsed Projects context drawer covers it directly, and Projects search replaces and restores the section tabs covers the search open/Escape/close paths.
  • Renamed empty/error states are consistently followed by the modified assertions in project-pr-review.spec.ts (punctuation-insensitive "No reviews yet" / "Could not load reviews") and useRetainedProjectGitViews.test.mjs (assert.match(/^No reviews yet/)).
  • Rendered-but-disabled owner gating (add-project-channel, add-project-repository with disabled={!canEdit}) is UI-affordance only; no privilege change. New spec assertion add-project-repository visible matches.

Stack reminder: base pt5 (#6597) still carries pre-squash ancestry; once #6597 lands, pt6 needs the same tree-identity-preserving rebase we did on #6595 (verify git diff old-head new-head empty).

Residuals: F4, F5, F6 tracked here as the durable sink; F1/F2 are blocking and owned by the implementer.

@thomaspblock
thomaspblock force-pushed the projects-channel-first-pt6-overview-polish branch 2 times, most recently from ef0d94a to 81ee2e0 Compare August 23, 2026 22:59
@thomaspblock

Copy link
Copy Markdown
Contributor Author

Cassandra exact-head adversarial delta review — 81ee2e01a18901e7aa18e9c074e348bdd6584ae1

Verdict: needs work. The recipient-impersonation finding from the prior head is fixed, but the rebuilt head retains one correctness blocker.

P1 / confidence 100 — repository-only communities are replaced by a false empty-project screen

Evidence from the exact head:

  • desktop/src/features/projects/ui/ProjectsView.tsx:118-122:
    const projectReadModels = projectsQuery.data ?? [];
    const projects = React.useMemo(
      () => projectReadModels.filter(isExplicitProject),
      [projectReadModels],
    );
  • desktop/src/features/projects/ui/ProjectsView.tsx:521-523:
    if (projects.length === 0) {
      return <EmptyState />;
    }
  • The repository tab is capable of rendering all read models at ProjectsView.tsx:312-325, but that code is unreachable when the relay has legacy/standalone kind-30617 repositories and no explicit kind-30621 project.
  • The resulting copy is affirmatively wrong at desktop/src/features/projects/ui/ProjectCards.tsx:312-315: No projects yet / Projects published to this relay will appear here.

Concrete trigger: a valid relay with one or more unclaimed NIP-34 repositories and zero kind-30621 project events. buildProjectReadModels preserves those repositories as legacy: true, then this view filters them all out and returns before mounting the Repositories tab. The user cannot browse those repositories from Projects even though the product contract explicitly requires: “a repo in no project still renders on its own, exactly as before” (VISION_PROJECTS.md:73).

Required correction: use the full read-model set to decide whether the Projects workspace is empty, while keeping explicit-only data where project-specific aggregation requires it. Add an E2E contract for a repository-only relay that reaches and renders the Repositories section.

Verified fixed / covered

  • Default selection now prefers only personaId === "builtin:fizz"; a display-name-only relay agent cannot preempt the sorted managed candidate, with regression coverage.
  • Restored SidebarProjectsSection, intended intro-copy assertion, compatibility-warning display, awaited project navigation, and 10-minute activity clock tick are present.
  • Inherited authority and navigation contracts are present: authoritative home binding, fixture buzz-channel, terminal project-home entry, #buzz browser assertions, and project-channel-home E2E assertion.
  • Search tokenization is render-safe (React text sinks), and selection drawer save/restore is edge-triggered with listener cleanup.

Fresh exact-head CI run 32672259386 is still in progress as of this report, so CI remains an additional merge gate. No merge-readiness verdict should be inferred until it completes green.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge delta review — PR #6600 rebuilt head 81ee2e01a18901e7aa18e9c074e348bdd6584ae1

Supersedes my report at bff5267d7 (comment 5388924562). Parent verified: single parent = repaired pt5 head 07d2ce6ffb94c902347ee1d5a67ee7808365cb90. GitHub MERGEABLE.

Verdict: merge-ready pending fresh exact-head CI. All prior blocking findings and residuals are fixed; the delta contains exactly those fixes and nothing else.

Delta scope (independently verified, not taken on trust)

I diffed the old pt6 delta (93814111e..bff5267d7) against the new pt6 delta (07d2ce6ff..81ee2e01a) line-by-line. After context-shift noise, the substantive delta-of-deltas is exactly six clusters:

  1. F2 fixedAppSidebarPinnedHeader.tsx:4 re-imports and :186 re-renders <SidebarProjectsSection />; the removal hunk is gone from the PR entirely.
  2. F1 fixedchannels.spec.ts:1752/1755 now assert "Add agent" / "Add an agent here." (spec updated to the intended copy rather than reverting the copy).
  3. Cassandra P1 fixedprojectAgentSelection.ts is now personaId === "builtin:fizz" ?? agents[0] ?? null; the display-name "fizz" || "kit" fallback is deleted. New spoof regression test: ignores an unmanaged agent using the Fizz display name (projectAgentSelection.test.mjs) asserts a bare { name: "Fizz" } loses to a managed candidate.
  4. F4 fixedProjectsView.tsx:694-701 restores the compatibilityWarning toast branch AND keeps await goProject(result.project.id).
  5. F5 fixedProjectsView.tsx:215 adds const activityDigestNow = useNow(600_000); digest memo now takes nowSeconds: Math.floor(activityDigestNow / 1_000) with activityDigestNow in deps — same tick cadence as the feed.
  6. Nothing else. No other file left or entered the 47-file set (only AppSidebarPinnedHeader.tsx dropped out of the diff and channels.spec.ts entered it, both explained above).

Carto's five Smoke(3) regression contracts — present at 81ee2e01a

  • hasAuthoritativeHomeBinding: grep hits in projectHomeChannel.ts (2), projectHomeChannel.test.mjs (3), ProjectDetailScreen.tsx (2)
  • fixture "buzz-channel" tag: e2eBridge.ts:5898, 5991
  • terminal-wheel.spec.ts:188: project-home-context-repo-buzz entry step present
  • channel-browser.spec.ts:83,107,133: /#buzz/ sort contracts present
  • entity-link-recipient-cards.spec.ts:582: project-channel-home assertion present

F6 residual (unchanged, still open, minor)

readStoredRepositoryScope/writeStored*Scope + ProjectsListScopeDropdown remain orphaned at this head (checked desktop/src at 81ee2e01a). Cleanup can trail the stack.

Remaining gate

CI at the exact SHA: Desktop Core, Smoke (1)–(4), Relay, Build (macOS) are in_progress as of this review; DCO / token guard / changed-paths already green. Fresh full-green at 81ee2e01a is the binding merge gate — including Smoke (3), which was cancelled in the prior run and carries the commit-detail/pr-review blast radius that was previously unmeasured.

Make the Projects overview section-aware, searchable, responsive, and consistent with sidebar and empty-state patterns while keeping list rendering bounded.

Signed-off-by: Thomas Petersen <thomasp@squareup.com>
Co-authored-by: Thomas Petersen <thomasp@squareup.com>
@thomaspblock
thomaspblock force-pushed the projects-channel-first-pt6-overview-polish branch from 81ee2e0 to 20dbe5a Compare August 23, 2026 23:07
@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge delta review — PR #6600 replacement head 20dbe5a4f6906c7a9435f95e6a9952a1867315a2

Supersedes my report at 81ee2e01a (comment 5388970328). Parent verified unchanged: 07d2ce6ffb94c902347ee1d5a67ee7808365cb90 (repaired pt5). GitHub MERGEABLE.

Verdict: merge-ready pending fresh exact-head CI. No new findings.

Delta scope (81ee2e01a..20dbe5a4f, verified with git diff — 3 files, +46/−23)

  1. ProjectsView.tsx:521 — early-return gate changed from projects.length === 0 to projectReadModels.length === 0. Correct: projects is the isExplicitProject-filtered subset, so a relay with only implicit/legacy repository read models previously hit the whole-view EmptyState and made the Repositories tab unreachable. Downstream checked: visibleRepositories (line 312) builds from projectReadModels, so the tab has content; visibleProjects (line 260) and the digest still use explicit projects, whose empty results render section-level empty states — not the whole-view gate. No path re-introduces the false-empty.
  2. e2eBridge.ts — new __BUZZ_E2E_REPOSITORY_ONLY_PROJECTS__ flag wraps only the kind-30621 project-buzz seed in a conditional; the fixture content inside (including the buzz-channel tag, lines 5979-6003) is byte-identical to what I verified at 81ee2e01a, just re-indented. Standalone kind-30617 repositories are retained.
  3. projects-v3-screenshots.spec.ts:67-82 — new behavior-level test repository-only relays keep the Repositories section available: enables the flag, opens Projects, clicks Repositories, asserts repository-card-buzz/repository-row-buzz visible. This is the test that would fail if the line-1 gate regressed — the fix ships with its own coverage.

Nothing else in the delta. All verifications from my 81ee2e01a report (five review fixes, Carto's five Smoke(3) contracts, delta-of-deltas purity vs bff5267d7) carry over unchanged — the three touched files do not intersect them except e2eBridge.ts, where I re-confirmed the buzz-channel fixture tag survived.

Remaining gate

Fresh CI at 20dbe5a4f is in_progress (Core, Smoke 1–4, Relay, macOS build); DCO/token-guard/changed-paths green. Full green at this exact SHA — including Smoke (3) — remains the binding merge gate. F6 (orphaned scope machinery) remains the only open residual, minor.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Cassandra replacement-head delta review — 20dbe5a4f6906c7a9435f95e6a9952a1867315a2

Code verdict: merge-ready; final verdict pending exact-head CI. No new security or correctness findings in the three-file delta from 81ee2e01a.

The prior repository-only P1 is closed:

  • desktop/src/features/projects/ui/ProjectsView.tsx:521 now gates the truly empty workspace on projectReadModels.length === 0, so implicit kind-30617 repository read models reach the overview.
  • The Repositories section continues to derive rows from projectReadModels, while project-only aggregation remains scoped to explicit projects.
  • The new E2E mode suppresses only kind-30621 seeds while retaining kind-30617 repository announcements, and the test navigates through the real Projects UI to the Repositories section and asserts the standalone buzz repository is visible.

I also checked the harness change for silent-pass risk: the flag is installed before installMockBridge/navigation; buildMockProjectEvents() conditionally omits the project announcement at store construction; repository announcements remain seeded. The test therefore exercises the reported production state rather than merely injecting a card.

Exact parent is still repaired #6597 head 07d2ce6ffb94c902347ee1d5a67ee7808365cb90; GitHub reports MERGEABLE/CLEAN. Exact-head CI run 32672690047 remains in progress. At this report, changed-path guards and macOS build are green; Desktop Core, Relay, and Smoke 1–4 are still running. Full green remains the binding merge gate.

Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge review #4 — CI triage at exact head 20dbe5a4f6906c7a9435f95e6a9952a1867315a2

Verdict: NOT merge-ready. Fresh exact-head CI (run 32672690047) completed: Desktop Core, Build, Relay, both Integration shards, Smoke (1) and (2) green; Smoke (3) failed (8 failed / 3 flaky / 256 passed) and Smoke (4) failed (1 failed / 8 flaky / 243 passed).

I diffed the failure set against the pt5 base's own CI at 07d2ce6ff (run 32671549361, Smoke (3) failed there too: 3 failed / 4 flaky; Smoke (4) green). Classification below separates what pt6 must fix from what pt5 owns.

A. pt6-caused — pass at base 07d2ce6ff, fail deterministically at head (blocking, confidence 100)

All failed all 3 retries at head and passed in the base Smoke (3) run.

  1. project-pr-review.spec.ts:916 — "preserve partial results from batched queries." Test body is byte-identical between base and head (verified by diff); passes at base (✓ 239 … 2.1s in base log). At head the work-item query #a filter is missing 30617:bb22a5…:design-system:

    - "30617:bb22a5299220cad76ffd46190ccbeede8ab5dc260faa28b6e5a2cb31b9aff260:design-system",
    

    (spec line 960: expect([...(filter["#a"] ?? [])].sort()).toEqual(expectedRepoAddresses)). The e2eBridge design-system seeds are unchanged (3 refs at both SHAs), so a pt6 source change dropped a project's repo addresses from work-item query enumeration — likely the ProjectsView/read-model rewrite. This is a real behavioral regression: work items for one project silently excluded from batched queries.

  2. project-pr-review.spec.ts:1195 — "project channels are grouped by project." getByTestId('projects-channel-project-group').nth(1).getByTestId('projects-group-select')… element not found (spec :1286 toHaveCSS("opacity", "0")). Passes at base. pt6 rewrote ProjectsChannelsList.tsx (117 lines changed) — the second project group or its select control no longer renders.

  3. project-pr-review.spec.ts:1397 — "project detail lists follow overview header geometry." project-discussion-channels-panel padding expected 16px, received 24px (spec :1476). Passes at base — pt6 changed panel geometry (projectPanelStyles.ts, ProjectsListHeaderBar.tsx both touched) without updating either the style or the spec to agree.

  4. project-pr-review.spec.ts:1669 — "overview presents collapsible context beside grouped activity." projects-overview-context-panel >> projects-overview-people not found (spec :1739). Passes at base; pt6 modified this test (+ hunks at 1705/1863) and ProjectsOverviewPanel.tsx/ProjectsOverviewRail.tsx — the people block no longer mounts inside the context panel.

  5. project-pr-review.spec.ts:2159 — "Projects search replaces and restores the section tabs." New pt6 test (absent at base). project-row-space-invaders-3d not visible (spec :2180). A brand-new pt6 test failing all retries on its first CI run means the search-restore flow was never verified against CI-rendered state.

  6. projects-v3-screenshots.spec.ts:276 (Smoke 4) — "projects v3 workspace screenshot states." Spec :764 asserts repositoryContextIcon opacity: 0.6; receives 1. pt6 replaced the Info icon (which had opacity-100/opacity-60 states) with DrawerPanelIcon in ProjectRightPanelControls.tsx and updated the opacity assertions in project-pr-review.spec.ts — but missed the same assertion in projects-v3-screenshots.spec.ts:764. Smoke (4) is green at base, red at head; this is the only deterministic Smoke (4) failure.

B. Base-inherited — fail identically at pt5 head 07d2ce6ff (not pt6's regression, but blocks the stack)

  • project-commit-detail.spec.ts:64projects-collection-group-label filtered "Mine" not found (spec :118). This testid exists in no source file at either SHA (whole-tree git grep at both: spec-only). Same for create-project-template / create-project-team below. The pt5 specs assert elements that nothing renders.
  • project-commit-detail.spec.ts:272 (base :274) — create-project-template expected "None", element not found (spec :284).
  • project-commit-detail.spec.ts:752 (base :753) — 30s timeout clicking [data-projects-text-priority="primary"] in sheetRow (spec :809).

These three fail all retries at both SHAs — they belong to pt5 (#6597) and must be fixed there (or in a pt6 commit that owns them explicitly). pt6 cannot go green while its base is red on the same shard.

C. Flaky at head (passed on retry — not blocking, noted for the record)

  • project-pr-review.spec.ts:1306 (flaky at base too), overscroll-boundary.spec.ts:34, persistent-agent-audience.spec.ts:205 (all flaky at base too). Smoke (4)'s 8 flaky (workflows.spec.ts cluster, video-attachment, workflow-local-controls) all passed on retry and are non-project specs — consistent with runner slowness, not pt6.

What I verified clean

Required before merge-ready

  1. Fix findings A1–A6 (pt6 commits).
  2. Resolve B-cluster in pt5 (feat(projects): unify navigation and reviewed channel requests #6597) or explicitly adopt into pt6 — Smoke (3) cannot pass with those in the shard.
  3. Fresh exact-head CI, all shards green, at whatever SHA results.

Evidence: head Smoke(3) job 97275942105 log; base Smoke(3) run 32671549361 job 97273114460; git grep/git diff at the two SHAs as cited inline.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Cassandra exact-head CI follow-up for de324a34adb2080d642612519ef5c048595ff0b9 (run 32674538188, Smoke (3) job 97280445891): not merge-ready.

The completed job reports 7 hard failures, 2 flaky, 258 passed, 1 skipped. All five Part 6 failures previously identified by Gauge remain deterministic through both retries:

  1. project-pr-review.spec.ts:916 — batched work-item enumeration still omits the expected repository address.
  2. project-pr-review.spec.ts:1195 — the second grouped channel's projects-group-select is absent.
  3. project-pr-review.spec.ts:1397project-discussion-channels-panel still computes 24px padding, expected 16px.
  4. project-pr-review.spec.ts:1669projects-overview-people is absent from the overview context panel.
  5. project-pr-review.spec.ts:2159project-row-space-invaders-3d is absent after restoring from search.

The two other hard failures match the inherited Part 5 failures:

  • project-commit-detail.spec.ts:256create-project-template absent (expected text None).
  • project-commit-detail.spec.ts:736 — commit-feed click times out at 30s.

The two tests that failed initially but passed on retry are outside the Part 6 Projects delta and remain classified as flakes:

  • onboarding-agent-defaults.spec.ts:908
  • persistent-agent-audience.spec.ts:205

The narrow stale-contract patch did what it claimed: the removed Mine and Local/Remote assertions no longer appear among failures. Merge readiness still requires implementation triage for the five Part 6 regressions, disposition of the two inherited hard failures (in #6597 or explicitly here), replacement-head review, and a fully green exact-head run.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge review #5 — delta review + CI at exact head de324a34adb2080d642612519ef5c048595ff0b9

Verdict: NOT merge-ready; converging. Supersedes review #4's CI snapshot (findings A1–A5 and two B-cluster items remain open at this head).

Delta review 20dbe5a4f..de324a34a — clean, no new findings

Spec-only, 2 files (+2/−18); parent = reviewed 20dbe5a4f:

  • projects-v3-screenshots.spec.ts:764/:800: removed opacity asserts on the deleted Info icon, replaced with repositoryPanelTab aria-pressed="false"/"true" — matches the canonical assertion style in project-pr-review.spec.ts. Closes A6.
  • project-commit-detail.spec.ts: removed the two projects-collection-group-label "Mine" assertions and the projects-row-location/repositories-row-location assertions — matching pt6's intentional removal of scope grouping and the location column. Closes B-item :64.

No app source touched.

CI at exact head (run 32674538188)

  • Smoke (4): green — A6 confirmed closed by CI, not just review.
  • Smoke (3): failed — 7 failed / 2 flaky / 258 passed. The failed set is exactly the review-docs(readme): clarify desktop setup #4 residue:
    • pt6-caused, still open (A1–A5): project-pr-review.spec.ts:916 (work-item #a filter drops 30617:bb22a5…:design-system — behavioral regression), :1195, :1397, :1669, :2159. Untouched by this delta.
    • base-inherited, still open: project-commit-detail.spec.ts:256 ("creating a project…", create-project-template element not found — testid exists in no source file at either SHA) and :736 (30s click timeout on [data-projects-text-priority="primary"]). Both fail identically at pt5 head 07d2ce6ff.
  • Flaky-passed-on-retry: onboarding-agent-defaults:908, persistent-agent-audience:205 — non-blocking.
  • All other shards green (Core, Build, Relay, Integration 1–2, Smoke 1–2).

Remaining gate

  1. Fix A1–A5 (code, or spec updates each justified against intentional pt6 behavior).
  2. Resolve the two base-inherited project-commit-detail failures in feat(projects): unify navigation and reviewed channel requests #6597 or adopt into pt6.
  3. Fresh exact-head CI, all shards green.

Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
@thomaspblock

Copy link
Copy Markdown
Contributor Author

Cassandra replacement-head review of 9d9593b658008a2a517195c1c152566ca78610fd (formed independently before consulting any new reviewer verdict): one blocking correctness finding; not merge-ready.

P1 — repository-only data is restored in the lists but still excluded from the context rail (confidence 100)

Evidence:

  • desktop/src/features/projects/ui/ProjectsView.tsx:118-122 defines the rail's projects input as only explicit projects:

    const projectReadModels = projectsQuery.data ?? [];
    const projects = React.useMemo(
    () => projectReadModels.filter(isExplicitProject),

  • The replacement correctly sends all read models to work-items and Channels at ProjectsView.tsx:149 and :843-845, but ProjectsView.tsx:598-609 still sends the filtered subset to the context rail:

    projects,
    summaries: activitySummariesQuery.data,

  • That summary query is itself fed only the filtered subset at ProjectsView.tsx:145:

    const activitySummariesQuery = useProjectActivitySummariesQuery(projects);

  • The context implementation derives repository and channel totals directly from that argument at projectsOverviewContext.ts:218-224:

    const channelCount = uniqueProjectRelatedChannelCount(projects);
    const repositories = repositoryCount(projects);
    and displays those values in the Repositories and Channels contexts at :226-249 and :255-277.

  • Repository people are commit authors from the same summaries at projectsOverviewContext.ts:199-207, so repository-only authors are also absent.

Concrete trigger: a community containing only a standalone kind-30617 repository with a bound channel now reaches the Projects surface, and the Repositories/Channels lists correctly show it after this replacement. However, the adjacent Repositories rail says 0 Repositories, the Channels rail says 0 Channels / 0 Repositories, and repository commit-author avatars cannot appear. A mixed community undercounts by every standalone repository/channel. This is the same explicit-project/read-model split that caused the earlier false global empty state, now left at the rail boundary.

The existing repository-only E2E checks list reachability/content, not these context totals or people, so the regression is not covered. The safe fix needs section-aware rail inputs: retain explicit projects for explicit-project counts/cards, while deriving repository/channel totals and repository activity people from all read models (or pass those aggregates explicitly). Merely changing the single projects rail prop wholesale may relabel standalone repositories as Projects, so that needs care.

Coverage with no additional findings

I reviewed the full five-file delta plus the surrounding work-item fan-out, channel grouping/collapse, profile lookup, people rendering, context derivation, standalone repository read-model construction, and the changed E2Es. The all-read-model work-item query itself is sound and deduplicates repository addresses; the restored people component does not introduce an injection boundary; the padding and orphan-assertion changes are scoped. The search assertion now exercises seeded data and both supported layouts, but is weaker than the prior exact row assertion; I found no correctness defect from that alone.

Residual/gate: exact-head CI is still running and remains binding after this finding is fixed. The inherited Part 5 Smoke (3) failure must still be resolved in #6597 or explicitly adopted before a green stacked run is possible.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge review #6 — delta review at exact head 9d9593b658008a2a517195c1c152566ca78610fd

Delta verdict: clean, no new findings. Merge verdict: pending exact-head CI (run 32676776753 in progress) — and note one known deterministic base-inherited failure this delta cannot fix (below).

Parent verified: single parent = reviewed de324a34a. Delta is 5 files (+22/−6), exactly the six announced fixes:

Findings closed (verified against source at this SHA)

  • A1 (work-item enumeration): closed. ProjectsView.tsx:149 now useProjectsWorkItemsQuery(projectReadModels) — matches the root cause I traced (fetchProjectsWorkItems fans out #a from projects.flatMap(p => p.repositories), projectWorkItems.ts:87-93). Repository-only repos are back in every work-item query. Wrench's scoping answer on activitySummariesQuery is sound: its consumers are explicit-project cards/context, and repository activity has its own useRepositoryActivitySummariesQuery(projectReadModels) path (ProjectsView.tsx:146-148). Accepted.
  • A2 (channels grouping): closed. ProjectsView.tsx:841 passes projectReadModels to ProjectsChannelsList, restoring repository-only bindings → the second projects-channel-project-group renders. Project collection cards stay explicit-only, consistent with the pt6 contract.
  • A3 (16px vs 24px): closed. DiscussionChannels.tsx:405 adds className="px-4" to the empty-state ProjectPanelState; cn() is tailwind-merge (shared/lib/cn.ts), so px-4 correctly displaces the component's default px-6 → computed 16px.
  • A4 (overview people): closed. ProjectsOverviewPanel.tsx:268-275 mounts ProjectsOverviewPeople (which carries data-testid="projects-overview-people", ProjectsOverviewRail.tsx:141) inside the context panel when context.people.length > 0; profiles plumbed from ProjectsView.tsx:214 via the new optional prop.
  • A5 (search restore): closed via justified spec fix. project-pr-review.spec.ts:2179-2183 now searches seeded buzz and accepts card or row presentation. Justified: space-invaders-3d exists only in unit-test fixtures (projectCollection.test.mjs), not in e2eBridge.ts (zero grep hits) — the old assertion referenced a project the e2e mock never seeded, so it could never pass. Searching real seeded data is the correct contract.
  • Carto's orphaned pair: closed. create-project-template/create-project-team assertions removed (spec :267-268 deleted); both testids exist nowhere in desktop/src at this SHA.

Verification posture

Wrench's focused runs (6/6 project-pr-review regression batch, 1/1 create-project contract, 5,462 desktop unit tests, check/typecheck) cover the changed behavior. Binding gate remains exact-head CI.

Known residual that will likely keep Smoke (3) red

project-commit-detail.spec.ts:734 ("commit detail opens from the commits feed with a diff", 30s click timeout on sheetRow [data-projects-text-priority="primary"]) is untouched by this delta and failed deterministically (×3 retries) at both pt5 head 07d2ce6ff and pt6 head 20dbe5a4f. Unless it is fixed in #6597 or adopted here, run 32676776753's Smoke (3) cannot go fully green even with all six fixes holding. The pt5-adoption fork Carto and I flagged is now the last structural blocker on this shard.

Will post the CI verdict when run 32676776753 completes.

Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
@thomaspblock

Copy link
Copy Markdown
Contributor Author

Cassandra re-review of exact replacement head f0bb9d37b0d6536fb50698e06a077018b9320fb4: P1 closed; no new findings; code merge-ready pending exact-head CI and stacked-base disposition.

I reviewed the full 9d9593b65..f0bb9d37b delta and retraced the context call path:

  • ProjectsView.tsx:607-611 now passes both explicit projects and full projectReadModels, plus repository-keyed activity summaries.
  • projectsOverviewContext.ts:231-237 uses all read models only for repository/channel aggregates while retaining explicit projects for Projects counts and project summaries.
  • projectsOverviewContext.ts:193-216 selects repository summaries only for the Repositories people pod; Tasks/Reviews continue to use work-item actors, Activity retains explicit-project summary semantics, and Channels intentionally exposes no people.
  • The repository activity hook deduplicates by repoAddress, and the model fold excludes claimed repositories from legacy read models, so the broader aggregate does not double-count normal explicit-project members.
  • The new regression covers the mixed case directly: 1 explicit project + 1 standalone repository yields Projects=1, Repositories=2, Channels=2 and includes the standalone commit author.

Independent focused verification at exact HEAD: projectsOverviewContext.test.mjs 10/10 passed. No security boundary changed; inputs remain relay-derived typed read models rendered through React.

Final gate remains fresh, fully green exact-head CI. Separately, the inherited #6597 commit-feed timeout still needs resolution in its owning branch or explicit adoption here before the stacked run can be called green.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge review #7 — delta review at exact head f0bb9d37b0d6536fb50698e06a077018b9320fb4

Delta verdict: clean, no new findings. Merge verdict: pending exact-head CI run 32677183349 (in progress). Supersedes review #6's CI gate; the prior run 32676776753 at 9d9593b65 was cancelled (superseded by this push), so it certifies nothing.

Parent verified: single parent = reviewed 9d9593b65. Delta is 4 files (+90/−5), scoped exactly to Cassandra's context-scope P1:

Verified

  • Dual-scope contract is explicit and correctly wired. projectsOverviewContext.ts:229 derives readModels = projectReadModels ?? projects; :235-236 count Channels/Repositories from readModels while summaryWorkItemCounts/owners stay on explicit projects — matching the announced contract (Projects count/owners explicit-only; repository/channel totals include repository-only models).
  • Repository people path. overviewContextPeople (projectsOverviewContext.ts:203-206) selects repositorySummaries only when filter === "repositories"; caller passes repositoryActivitySummariesQuery.data (ProjectsView.tsx:610), whose query is enabled exactly on the repositories tab (ProjectsView.tsx:146-148) — so standalone commit authors appear on the Repositories pod and nowhere else. latestCommitAuthors iterates Object.values, so summary record keying is not load-bearing.
  • Memo correctness. ProjectsOverviewPanel.tsx:199-216 adds both new inputs to the React.useMemo dependency array — no stale-context hazard.
  • Regression test quality. The new unit test constructs one explicit project + one repository-only read model and asserts Repositories=2, Channels=2, Projects=1, and the standalone commit author on the repositories pod — this is the test that fails if either count regresses to explicit-only. Independently ran the suite at this exact head: 10/10 pass (node --import ./test-loader.mjs --experimental-strip-types --test src/features/projects/ui/projectsOverviewContext.test.mjs).
  • Back-compat. projectReadModels/repositorySummaries are optional on OverviewContextInput with ?? projects fallback — other callers unaffected.

Open items (unchanged from review #6)

  1. Binding gate: exact-head CI 32677183349 full green.
  2. Base-inherited: project-commit-detail.spec.ts:734 (commit-feed 30s click timeout) — untouched, deterministic at pt5 head 07d2ce6ff and every pt6 head since. Must be fixed in feat(projects): unify navigation and reviewed channel requests #6597 or adopted here, or Smoke (3) stays red.

Will post the CI classification when the run completes.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge review #8 — CI classification at exact head f0bb9d37b0d6536fb50698e06a077018b9320fb4

Verdict: all pt6-caused findings are closed and CI-confirmed. The only remaining hard failure is the base-inherited #6597 defect. #6600 is merge-ready conditional on resolving that one inherited failure (fix in #6597 + rebase, or adopt into pt6) plus a final green run at whatever SHA results.

CI run 32677183349 (completed: failure — one job)

  • Green: Desktop Core, Build (macOS), Relay, Integration (both shards), Smoke (1), (2), (4).
  • Smoke (3): 1 failed / 3 flaky / 263 passed — down from 7 failed at de324a34a and 8 at 20dbe5a4f.

The one hard failure

project-commit-detail.spec.ts:734 — "commit detail opens from the commits feed with a diff", locator.click 30s timeout on sheetRow [data-projects-text-priority="primary"], all 3 retries. This is the base-inherited defect: it failed identically (same locator, same timeout, ×3) at pt5 head 07d2ce6ff's own Smoke (3) run (32671549361) before pt6's rewrite was ever measured. Not caused by, and not fixable by review of, this PR's delta.

CI-confirmed closures at this head

All five pt6-caused project-pr-review failures (A1–A5: :916 enumeration, :1195 channel grouping, :1397 padding, :1669 overview people, :2159 search restore) now pass in CI, as do both formerly-failing project-commit-detail tests (:64 metadata, :256 create-project) — the stale-contract and read-model fixes hold under the full shard, not just focused runs. Smoke (4) green confirms the screenshots-spec fix. Cassandra's context-scope P1 fix also survives its first full-shard measurement.

Flaky (passed on retry; not blocking)

messaging.spec.ts:2049, onboarding-agent-defaults.spec.ts:908, project-pr-review.spec.ts:1306 — all with prior flake history on this shard at earlier heads/base.

Remaining path to merge

  1. Fix :734's root cause in feat(projects): unify navigation and reviewed channel requests #6597 (owning branch) and rebase pt6, or adopt the fix into pt6 explicitly.
  2. One more exact-head run: full green including Smoke (3).

No open findings against pt6's own content. Reviews #3#7 delta chain (07d2ce6ff → 20dbe5a4f → de324a34a → 9d9593b65 → f0bb9d37b) is fully verified with each parent reviewed.

Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
@thomaspblock
thomaspblock marked this pull request as ready for review August 24, 2026 01:05
@thomaspblock

Copy link
Copy Markdown
Contributor Author

Cassandra delta review of exact head 344bf64f1da78c25ec6e112a3b2952f14c7e533c: clean; no new findings; merge-ready pending fresh exact-head CI.

The delta from reviewed f0bb9d37b is one test-only interaction change in desktop/tests/e2e/project-commit-detail.spec.ts:787-793: it replaces generic Escape with a click on the visible dialog-scoped accessible Close button.

I traced the surrounding implementation rather than accepting the locator alone:

  • The create action is owned by ProjectHomeWorkspaceSheet, which keeps its own issue-dialog open state and the sheet row being exercised afterward.
  • CreateIssueDialog delegates to CreateProjectWorkItemDialog, whose ChooserDialogContent uses the shared DialogContent built-in close control.
  • desktop/src/shared/ui/dialog.tsx:115-130 renders that control as a Radix DialogPrimitive.Close with screen-reader text Close, so the new role/name locator targets the actual supported close path and is scoped to create-issue-dialog.
  • Closing through that control drives onOpenChange(false) without turning the test into an implementation shortcut; the subsequent row click still verifies that the workspace sheet remains usable and the test continues through issue detail, repository navigation, commits feed, and diff detail.

This does not weaken application coverage or hide the original timeout. It removes a keyboard/focus side effect unrelated to the test's stated commit-detail contract. No application code or security boundary changed. Diff check is clean and remote/local exact head matched during review.

Fresh exact-head CI run 32678638549 is now the only remaining gate; the prior run cannot be used because this commit changes the formerly hard-failing test interaction.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge review #9 — delta review at exact head 344bf64f1da78c25ec6e112a3b2952f14c7e533c

Delta verdict: clean, no new findings. Final gate: exact-head CI run 32678638549 (in progress) full green.

Parent verified: single parent = reviewed f0bb9d37b. Delta is 1 spec file (+4/−1), no app code — exactly the adoption of the last inherited Smoke (3) failure (project-commit-detail.spec.ts:734).

Verified

  • Root cause agreement. Wrench's diagnosis matches my independent artifact trace (thread msg 249d1b3f, from the CI error-context snapshots at run 32677183349): after Escape, the accessibility tree shows plain project home — no workspace sheet, no issue rows — so the :791 click waited on a removed element. Same conclusion from two independent analyses.
  • Locator resolves. New locator getByTestId("create-issue-dialog").getByRole("button", { name: "Close" }): the testid comes from CreateProjectWorkItemDialog.tsx:56,108 (create-${itemName}-dialog, itemName="issue"); ChooserDialogContentDialogContent defaults showCloseButton = true (shared/ui/dialog.tsx:73) and the close button carries <span className="sr-only">Close</span> (dialog.tsx:129) — accessible name "Close" resolves.
  • Contract preserved. The test still asserts dialog dismissal (toHaveCount(0)) then exercises the intended continuation (open issue detail from the sheet row). Closing via the dialog's own control keeps the sheet mounted — this is the behavior the test intends to cover.
  • Wrench's focused run: 1/1 pass at this exact head; typecheck + push gates green (his report).

Residual (non-blocking, filed for product decision — not a pt6 regression)

The underlying app behavior remains: pressing Escape with the create dialog open dismisses both the dialog and the workspace sheet (double-dismiss via Radix focus restoration). The spec now routes around it rather than asserting either intended behavior. If one-Escape-one-layer is the intended UX, that's an app fix in the sheet/dialog Escape layering (useEscapeKey yields only on defaultPreventedshared/hooks/useEscapeKey.ts:21); if double-dismiss is intended, no action. Either way it predates this stack (identical failure at pt5 head 07d2ce6ff) and should not block #6600.

State of the review chain

07d2ce6ff → 20dbe5a4f → de324a34a → 9d9593b65 → f0bb9d37b → 344bf64f1 — every parent reviewed, every delta verified. No open findings against #6600's content. If run 32678638549 comes back full green including Smoke (3), my verdict is merge-ready.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge — CI classification, run 32678638549 @ exact head 344bf64f1 (final-gate run)

Verdict: NOT merge-ready yet. One base-inherited test race escalated from flaky to hard failure. The :734 adoption is confirmed fixed on CI.

Run status (completed: failure)

Job Result
Smoke (1), (2), (4) ✅ green
Desktop Core, macOS Build, Relay, Integration (1/2, 2/2) ✅ green
Smoke (3) ❌ 1 failed, 1 flaky

✅ Confirmed fixed

  • project-commit-detail.spec.ts:734passed in 5.2s (✓ 212). Wrench's Escape→Close-button fix is CI-confirmed. Confidence: 100.

❌ Hard failure — base-inherited, NOT caused by pt6

  • project-pr-review.spec.ts:1306 "overview tasks and reviews are grouped and selectable by project" — failed all 3 attempts with an identical signature:
    Locator:  getByTestId('projects-issue-project-group').first().locator('[data-testid^="projects-issue-row-"]')
    Expected: 30
    Received: 56
    at desktop/tests/e2e/project-pr-review.spec.ts:1347:24
    
  • Inheritance evidence (confidence 100): the pt5 base run at 07d2ce6ff (job 97273114460) shows the exact same failure — Expected: 30 / Received: 56 at :1347 — where it passed on retry Dependency Dashboard #1 (flaky bucket). It also flaked at f0bb9d37b (job 97287469655). The pt6 delta f0bb9d37b..344bf64f1 is 4 lines in project-commit-detail.spec.ts only, so it cannot influence this spec.
  • Root cause (confidence 75, quoted): the test snapshots the row count while data is still streaming, then asserts the same count after a collapse/expand round-trip:
    const rowCount = await rows.count();      // captured at 30, group still loading
    ...
    await expect(rows).toHaveCount(0);        // collapse
    await toggle.click();
    await expect(rows).toHaveCount(rowCount); // group now fully loaded: 56 ≠ 30
    (project-pr-review.spec.ts ~:1327–:1347). Same count-before-stable class as the previously adopted fixes. A stabilization fix: wait for the row count to settle (or for the overview loaded state) before capturing rowCount.

⚠️ Flaky (passed on retry, non-blocking)

  • overscroll-boundary.spec.ts:34 "locks viewport rubber-band outside conversation scrollers" — new to the flake list for this PR.

Options

  1. Adopt a stabilization fix for :1306 (recommended — 3/3 deterministic failure here means a bare re-run is a coin flip at best; base evidence says it can pass on retry, but the failure rate is worsening).
  2. Re-run Smoke (3) and hope the retry-pass behavior from the base run recurs — not recommended given 0/3 attempts passed in this run.

Strict gate holds: merge-ready requires fresh full-green CI at the exact head SHA, all shards including Smoke (3).

Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
@thomaspblock

Copy link
Copy Markdown
Contributor Author

Cassandra delta review of exact head 6eac8f18877f60aaf6989f1c16a6034ff3429b7e: clean; no new findings; merge-ready pending fresh exact-head CI.

I reviewed the complete two-file 344bf64f1..6eac8f188 delta and traced the progressive-mount data flow:

  • ProjectsIssuesList and ProjectsPullRequestsList build allGroups with each group's complete selectionItems, then use sliceGroupedRows to truncate only rows to the current incremental budget.
  • sliceGroupedRows spreads the original group and replaces only rows, so even a partially mounted group retains its authoritative full selectionItems array.
  • ProjectSelectableGroup receives that full array as items; exposing items.length as data-project-group-size therefore reports the complete selectable group size, not the current DOM window.
  • The E2E's web-first toHaveCount(authoritative size) waits for useIncrementalMount to finish its requestAnimationFrame growth before snapshotting rowCount. Collapse/expand and select/clear assertions remain unchanged, so behavior coverage is strengthened rather than bypassed.

The attribute is numeric React-owned metadata, not user-interpreted HTML or an authorization surface. It does not expose secrets, influence selection behavior, add sleeps/timeouts, or alter production rendering. It is also meaningful for every existing ProjectSelectableGroup caller because each caller already provides the same complete items collection used by group selection.

Diff check is clean; local and PR head matched during review. Fresh exact-head CI run 32680631241 is the binding final gate.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge — review #10, exact head 6eac8f18877f60aaf6989f1c16a6034ff3429b7e (delta 344bf64f1..6eac8f188)

Verdict: clean — no blocking findings. Root cause confirmed independently; the fix waits on the authoritative source count instead of snapshotting an intermediate mount state. Binding gate is fresh CI run 32680631241 (in progress at this head).

Root cause independently confirmed (confidence 100)

Wrench's "progressive row mounting, not query streaming" diagnosis matches my source trace exactly:

  • ProjectsIssuesList.tsx:211-218 builds selectionItems from the full group.rows inside the grouping memo, before any trimming.
  • sliceGroupedRows (useIncrementalMount.ts:62-79) trims via { ...group, rows: group.rows.slice(0, remaining) } — the spread preserves the untrimmed selectionItems.
  • useIncrementalMount(countGroupedRows(allGroups), 30, 60, …) (ProjectsIssuesList.tsx:224-229) renders the first 30 rows in the first commit; the first group's true size was 56. The old test snapshotted rows.count() in that window — deterministic 30-vs-56 under CI timing, exactly the failure signature.

Fix verified

  • ProjectSelectableGroup.tsx:47 adds data-project-group-size={items.length}items is the authoritative unsliced set (per above), so the attribute is the correct settled-count oracle. Presentation-only data-* attribute; zero behavior change; no other consumer affected (ProjectsChannelsList, ProjectWorkItemGroup just gain an inert attribute).
  • project-pr-review.spec.ts:1337-1339 web-first await expect(rows).toHaveCount(Number(await firstGroup.getAttribute("data-project-group-size"))) before capturing rowCount. Convergence is guaranteed: the useIncrementalMount rAF/transition loop grows until count >= total (useIncrementalMount.ts:39-52), and the first group has slice priority, so rendered rows reach the attribute value. No sleeps, no timeout inflation, no weakened assertions — collapse/expand and select-all contracts unchanged.
  • PR section covered identically: ProjectsPullRequestsList.tsx:218-247 has the same full-selectionItems + slice structure, and the attribute lives in the shared group component, so both loop iterations benefit.
  • Guard integrity: if the attribute were ever missing, Number(null) = 0 → toHaveCount(0) fails against rendered rows, and the following expect(rowCount).toBeGreaterThan(0) (:1341) backstops the empty case. No silent-pass path.

Residuals (non-blocking)

  • [50] The getAttribute read is one-shot inside the retried assertion: if the underlying work-items query were still appending data at that instant, the captured size could go stale and the wait could time out (rows would grow past it: 30 → min(30+60, total) skips intermediate values). CI evidence says this doesn't happen here — the failing run's locator resolved to 56 for all 14 polls, i.e., source data was already settled — so this is a theoretical flake mode, strictly better than the one it replaces.
  • [25] Pre-existing, outside this delta: the collapsed-header count renders count={group.rows.length} from the sliced group (ProjectsIssuesList.tsx:304), so a header collapsed mid-mount could briefly understate the group size. Transient, self-corrects as the mount counter grows; noting for completeness, not for this PR.

What I verified / didn't

  • Verified: full delta read (4 lines, 2 files), mechanism traced in both list components + shared group + hook at exact head via git show; PR head SHA matches announcement.
  • Not independently re-run: Wrench's 10×/3× focused e2e repeats and the 5,463-test unit suite (claims noted; the delta cannot affect unit behavior — attribute-only app change). CI run 32680631241 is the binding evidence.

Merge gate unchanged: fresh full-green CI at exact head 6eac8f188, all shards including Smoke (3). I'll classify when it completes.

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Cassandra final verdict for PR #6600: MERGE-READY at exact head 6eac8f18877f60aaf6989f1c16a6034ff3429b7e.

I independently verified that PR head still equals the reviewed SHA and binding run 32680631241 completed successfully at that same headSha. All applicable jobs are green, including Desktop Core, macOS build, Smoke (1)–(4), Desktop E2E Relay, both integration shards, and the Desktop aggregate.

No open security or correctness findings remain. The repository-only empty-state and context-scope P1s are closed with behavior/unit coverage; all Part 6 deterministic regressions are closed; the inherited Smoke (3) failures adopted here are now CI-confirmed. Residual UX/test-hardening notes already recorded in earlier durable review comments are non-blocking and do not change this verdict.

CI: https://github.com/block/buzz/actions/runs/32680631241

@thomaspblock

Copy link
Copy Markdown
Contributor Author

Gauge — final CI classification & merge-ready verdict, run 32680631241 @ exact head 6eac8f188

Verdict: MERGE-READY. The strict gate is satisfied: fresh full-green CI at the exact head SHA 6eac8f18877f60aaf6989f1c16a6034ff3429b7e (verified via headSha on the run), all shards including Smoke (3).

Run 32680631241 (completed: success)

Job Result
Smoke (1)(2)(3)(4) ✅ all green
Desktop Core, macOS Build, Relay E2E, Integration (1/2, 2/2) ✅ green

Smoke (3) detail (job 97296702656): 264 passed / 3 flaky / 1 skipped / 0 failed

  • project-pr-review.spec.ts:1306✓ passed first attempt in 3.3s (✓ 243). The data-project-group-size settled-count wait closed the 30-vs-56 incremental-mount race. Confidence: 100.
  • project-commit-detail.spec.ts:734✓ passed first attempt in 5.4s (✓ 214). The Escape→Close fix holds across runs.
  • Flaky (passed on retry, all with prior history, non-blocking): messaging.spec.ts:2049, overscroll-boundary.spec.ts:34, persistent-agent-audience.spec.ts:205.

Review-chain summary (pt6, every head reviewed)

07d2ce6ff (pt5 base) → 20dbe5a4fde324a34a9d9593b65f0bb9d37b344bf64f16eac8f188. All pt6-caused findings (A1–A6, context-scope P1) closed and CI-confirmed; both adopted base-inherited stabilizations (:734, :1306) fixed at root cause and green under the full shard.

Residuals on record (non-blocking, already filed)

  1. App-side Escape double-dismiss (dialog + workspace sheet in one keypress) — routed around by the :734 fix, not resolved; predates this stack (comment 5389570320).
  2. One-shot data-project-group-size read could theoretically go stale under a still-appending source [50] (comment 5389797154).
  3. Collapsed-header count uses the sliced group mid-mount [25] — pre-existing, cosmetic-transient (comment 5389797154).

No open findings against #6600's content. From the correctness/testing/reliability lens, this PR is clear to merge.

@thomaspblock
thomaspblock merged commit f0192ea into projects-channel-first-pt5-navigation-requests Aug 24, 2026
23 checks passed
@thomaspblock
thomaspblock deleted the projects-channel-first-pt6-overview-polish branch August 24, 2026 02:14
thomaspblock added a commit that referenced this pull request Aug 24, 2026
## Summary
- seed new project homes with a built-in operational canvas while
preserving custom templates and None
- make category create actions open the matching project-aware modal and
keep selection presentation type-specific
- aggregate Tasks and Commits across every repository with responsive
project/repository grouping and a repository Overview tab

This is the final part of the channel-first Projects stack, based on
#6600.

## Testing
- focused project template, work-item aggregation, and overview-context
tests: 16/16 passed
- Desktop unit suite: 5,455/5,455 passed
- E2E-mode Desktop build passed
- TypeScript, Biome, and differential file-size checks passed
- full pre-push gate passed

## Post-Deploy Monitoring & Validation
- create a project, inspect its canvas, and exercise Tasks/Commits
across a project with several repositories
- healthy signals: populated project canvas, one chronological commit
list with repository attribution, and complete task aggregation
- failure signals: missing canvas, single-repository-only counts,
duplicate repository groups, or category actions opening the wrong flow;
mitigate by reverting this PR

---------

Signed-off-by: Thomas Petersen <thomasp@squareup.com>
Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
matt2e pushed a commit that referenced this pull request Aug 25, 2026
## Summary
- make the Projects overview follow the active section with consistent
context, empty states, and row alignment
- add an animated section-local search that preserves sort and
responsive tab behavior
- simplify sidebar and list controls while keeping large project
collections incrementally mounted

This is Part 6 of the channel-first Projects stack, based on #6597. The
final part contains project workspace aggregation and the default
project-home template.

## Testing
- focused search, activity digest, agent selection, related-channel, and
overview-context tests: 26/26 passed
- Desktop unit suite: 5,451/5,451 passed
- E2E-mode Desktop build passed
- TypeScript, Biome, and differential file-size checks passed
- full pre-push gate passed

## Post-Deploy Monitoring & Validation
- exercise every Projects overview section, search transition, selection
state, and right context rail in staging Desktop
- healthy signals: stable tabs, aligned rows, correct section context,
and responsive large-list interaction
- failure signals: search flicker, stale context, wrapped toolbar rows,
missing empty actions, or input lag; mitigate by reverting this PR

---------

Signed-off-by: Thomas Petersen <thomasp@squareup.com>
Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
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