Run standup from a presenter board for the whole workspace - #102
Conversation
…pace The old standup was a per team ceremony room. It was scoped to one team when a standup covers the workspace, every interaction wrote to the database and then forced a refetch so each click cost two round trips, and the issues it listed were inert text, so the facilitator could not open the work being discussed. /standup is now a read only presenter board. One row of person tiles across the top for everybody in the workspace, and the selected person's work in three columns: closed since the last standup, in progress, and up next. Clicking a tile or pressing left and right moves between people as pure local state, so the board loads with a single request and issues none for the rest of the meeting. Issues are real rows that open the peek. There is no attendance step, no facilitator, no turn and no required notes, because the meeting happens on a call and Fireflies already records it. A timer is there for anybody who wants to keep the round moving. The board reads one windowed query over issues, capped per person per column so fresh backlog cannot push somebody's stale in progress work out of sight, with the exact counts fetched separately so a capped column says how many it is showing rather than quietly lying. The window start is the viewer's own previous working day, clamped server side. The ceremony REST routes and room components go with it. The scrum tables and standup-service stay, because the MCP scrum tools call that service directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
imshashank has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reached
Next review available in: 18 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR replaces the ceremony-based standup flow with a board that retrieves filtered issues and workload data. It adds core board services, an API route, query integration, realtime invalidation, and new board UI components. Legacy standup routes and components are removed. ChangesStandup board
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant StandupBoard
participant useStandupBoard
participant BoardRoute
participant standupBoard
StandupBoard->>useStandupBoard: request board data
useStandupBoard->>BoardRoute: GET /api/standup/board
BoardRoute->>standupBoard: load authenticated board
standupBoard-->>BoardRoute: return issues and workload
BoardRoute-->>useStandupBoard: return validated payload
useStandupBoard-->>StandupBoard: render board
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
apps/web/tests/lib/realtime/delta-bridge.test.tsx (1)
281-286: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing the duplicated standup assertion.
The test at Lines 281-286 asserts that
seencontains[STANDUP_ROOT]after the sameaction()used at Line 205. Line 206 already asserts the exact invalidation list, which includes[STANDUP_ROOT]. This test cannot fail unless Line 206 also fails. The second test in this block (Lines 288-296) is the new coverage that is worth keeping.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/tests/lib/realtime/delta-bridge.test.tsx` around lines 281 - 286, Remove the redundant `refreshes the board when somebody else moves an issue` test and its duplicated `STANDUP_ROOT` assertion, since the exact invalidation list is already covered by the existing test around `action()` and `trackInvalidations`; retain the distinct second test in this block.apps/web/tests/lib/query/use-standup-board.test.tsx (1)
107-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen the
keepPreviousDataassertion.
stubFetchat Line 108 returns the same body for every request. Afterrerenderwith a newsince, the assertion at Line 122 passes whether the data is retained from the first key or freshly resolved for the second key. The test cannot fail ifplaceholderData: keepPreviousDatais removed from the hook.Return a distinct payload for the second window, then assert the first window's data is still present before the second request resolves.
💚 Proposed change
- stubFetch({ since: SINCE.toISOString(), issues: [issue()], workload: [] }); + const later = new Date('2026-06-05T07:00:00.000Z'); + globalThis.fetch = mock((input: string | URL | Request) => { + const first = String(input).includes(encodeURIComponent(SINCE.toISOString())); + return Promise.resolve( + new Response( + JSON.stringify({ + since: SINCE.toISOString(), + issues: first ? [issue()] : [issue({ id: 'issue_2' }), issue({ id: 'issue_3' })], + workload: [], + }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ), + ); + }) as unknown as typeof fetch;Keep the existing assertion at Line 122 for the retained single issue, then await the second window and assert two issues.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/tests/lib/query/use-standup-board.test.tsx` around lines 107 - 124, Update the useStandupBoard test so stubFetch returns distinct payloads for the initial and updated since windows. After rerendering, assert the first window’s single issue remains visible before the second request resolves, then await the second window’s completion and assert its distinct payload contains two issues.apps/web/tests/features/standup/person-work.test.tsx (2)
119-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the avatar's accessible fallback.
ada.imageis null at Line 24, soPersonWorkLine 41 renders theAvatarfallback. No test asserts that the fallback exposes the member name to assistive technology. Add an assertion forrole="img"witharia-labelequal to the member name.💚 Proposed addition
expect(screen.getByTestId('standup-person').textContent).toBe('Ada Lovelace'); expect(screen.getByTestId('standup-person-summary').textContent).toBe('3 in progress, 9 open'); + expect(screen.getByRole('img', { name: 'Ada Lovelace' })).toBeDefined();Based on learnings: in the Orbit web UI, Avatar components must expose an accessible fallback with
role="img"andaria-label={name}when no image is available.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/tests/features/standup/person-work.test.tsx` around lines 119 - 127, Extend the test case around renderWork and the standup-person assertions to verify the image fallback exposes role="img" with aria-label "Ada Lovelace".Source: Learnings
85-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCreate the QueryClient once per test, not once per render.
ProviderscallscreateQueryClient()inside the component body. Every render ofProvidersbuilds a newQueryClientand discards the previous cache. Hoist the client so a re-render keeps the same instance.♻️ Proposed change
function Providers({ children }: { children: ReactNode }) { - return <QueryClientProvider client={createQueryClient()}>{children}</QueryClientProvider>; + const [client] = useState(createQueryClient); + return <QueryClientProvider client={client}>{children}</QueryClientProvider>; }Import
useStatefromreactfor this change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/tests/features/standup/person-work.test.tsx` around lines 85 - 87, Update the Providers component to preserve a single QueryClient instance across re-renders by initializing it with React useState, importing useState from react, and passing that stable client to QueryClientProvider instead of calling createQueryClient() directly during render.apps/web/src/features/standup/standup-board.tsx (1)
178-194: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider keeping the last good board visible when a refetch fails.
useStandupBoardsetsplaceholderData: keepPreviousData. After a successful load, a failed refetch setsboard.isErrorwhileboard.datastill holds the previous board. Line 180 then replaces readable content with the "Could not load the board" empty state.Show the error branch only when there is no data to display, and surface a non-blocking retry otherwise.
♻️ Proposed change
- if (failed) { + if (failed && people.length === 0) {A cleaner form passes
hasDatafromStandupBoard(board.data !== undefined) and gates the branch onfailed && !hasData.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/features/standup/standup-board.tsx` around lines 178 - 194, Update StandupBoard’s failed-state handling to show the blocking EmptyState only when the board has no data, using board.data !== undefined (or an equivalent hasData value) to gate failed. When previous board data exists during a failed refetch, keep rendering the board and surface a non-blocking retry affordance instead.apps/web/src/features/standup/standup-skeleton.tsx (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider announcing the loading state to assistive technology.
BoardSkeletonreplaces the whole board body and the whole route during navigation. It exposes no accessible name and no busy state. Screen reader users hear nothing while the board loads. Addrole="status"andaria-labelso the loading state is announced once.♿ Proposed change
- <div className="flex min-h-0 flex-1 flex-col" data-testid="standup-skeleton"> + <div + className="flex min-h-0 flex-1 flex-col" + role="status" + aria-label="Loading the standup board" + data-testid="standup-skeleton" + >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/features/standup/standup-skeleton.tsx` at line 9, Update the root container in the standup skeleton component to include role="status" and a descriptive aria-label indicating that the board is loading, so assistive technology announces the loading state.apps/web/src/features/standup/person-work.tsx (1)
41-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse one fallback name for a missing member.
Line 41 passes
'Unknown'toAvatar, and Line 44 renders'Unknown member'. The avatar's accessible label and the visible heading then disagree for the same person. Use the same string in both places.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/features/standup/person-work.tsx` around lines 41 - 44, Use one consistent fallback name for missing members in the Avatar name prop and the standup-person heading. Update the differing fallback in the member display block so both use the same string while preserving the existing member.name behavior.apps/web/tests/features/standup/buckets.test.ts (1)
38-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider narrowing
categoryand sharing this helper.
statetypescategoryasstring. A typo such as'complete'instead of'completed'would compile and silently move an issue into the up-next bucket, becausebucketIssuestreats every unrecognized category as up-next. Use theWorkflowState['category']type so the compiler rejects invalid categories.The identical helper exists at
apps/web/tests/features/standup/person-work.test.tsxLines 10-12. Extract it into a shared test helper.♻️ Proposed change
-function state(id: string, category: string): WorkflowState { +function state(id: string, category: WorkflowState['category']): WorkflowState { return { id, teamId: 'team_eng', name: id, category, color: '`#666666`', position: 0 }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/tests/features/standup/buckets.test.ts` around lines 38 - 40, Update the state helper to type category as WorkflowState['category'] so invalid workflow categories are rejected, and extract the identical state helper into a shared test utility reused by buckets.test.ts and person-work.test.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/features/standup/person-work.tsx`:
- Around line 101-111: Update the header in the person-work column rendering so
the standalone counted value is rendered only when issues.length is not less
than counted; retain the capped “showing {issues.length} of {counted}” element
and existing test identifiers unchanged.
In `@apps/web/src/features/standup/standup-board.tsx`:
- Line 107: Clarify the intended timer scope in the standup board: for
per-person timing, replace the mount-only startedAt value with state that resets
when activeId changes and pass it to StandupTimer; for whole-standup timing,
remove the activeId-based key so StandupTimer remains mounted. Preserve the
selected behavior consistently in the timer rendering flow.
- Around line 49-50: Update the active selection logic around activeId and
activeIndex to use selectedId only when it still exists in people; otherwise
fall back to people[0]?.id or null. Ensure activeIndex is derived from this
resolved roster selection so removed members never produce -1 or render as an
unknown member.
- Around line 77-81: Update the `useHotkey` registration for the `t` timer
toggle to include `enabled: peekId === null`, using the existing `peekId` state
so the hotkey is disabled while `IssuePeek` is open and remains enabled
otherwise.
In `@apps/web/tests/app/api/standup-board.test.ts`:
- Around line 21-27: Update the standup board route handler to parse
searchParamsOf(request) with standupBoardQuerySchema before passing the
validated input to standupBoard. In the standupBoard mock, remove schema parsing
and record input directly in received so the existing assertion validates
route-level request parsing.
---
Nitpick comments:
In `@apps/web/src/features/standup/person-work.tsx`:
- Around line 41-44: Use one consistent fallback name for missing members in the
Avatar name prop and the standup-person heading. Update the differing fallback
in the member display block so both use the same string while preserving the
existing member.name behavior.
In `@apps/web/src/features/standup/standup-board.tsx`:
- Around line 178-194: Update StandupBoard’s failed-state handling to show the
blocking EmptyState only when the board has no data, using board.data !==
undefined (or an equivalent hasData value) to gate failed. When previous board
data exists during a failed refetch, keep rendering the board and surface a
non-blocking retry affordance instead.
In `@apps/web/src/features/standup/standup-skeleton.tsx`:
- Line 9: Update the root container in the standup skeleton component to include
role="status" and a descriptive aria-label indicating that the board is loading,
so assistive technology announces the loading state.
In `@apps/web/tests/features/standup/buckets.test.ts`:
- Around line 38-40: Update the state helper to type category as
WorkflowState['category'] so invalid workflow categories are rejected, and
extract the identical state helper into a shared test utility reused by
buckets.test.ts and person-work.test.tsx.
In `@apps/web/tests/features/standup/person-work.test.tsx`:
- Around line 119-127: Extend the test case around renderWork and the
standup-person assertions to verify the image fallback exposes role="img" with
aria-label "Ada Lovelace".
- Around line 85-87: Update the Providers component to preserve a single
QueryClient instance across re-renders by initializing it with React useState,
importing useState from react, and passing that stable client to
QueryClientProvider instead of calling createQueryClient() directly during
render.
In `@apps/web/tests/lib/query/use-standup-board.test.tsx`:
- Around line 107-124: Update the useStandupBoard test so stubFetch returns
distinct payloads for the initial and updated since windows. After rerendering,
assert the first window’s single issue remains visible before the second request
resolves, then await the second window’s completion and assert its distinct
payload contains two issues.
In `@apps/web/tests/lib/realtime/delta-bridge.test.tsx`:
- Around line 281-286: Remove the redundant `refreshes the board when somebody
else moves an issue` test and its duplicated `STANDUP_ROOT` assertion, since the
exact invalidation list is already covered by the existing test around
`action()` and `trackInvalidations`; retain the distinct second test in this
block.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2787180c-c58a-478c-a1c4-09929fb45ed5
📒 Files selected for processing (43)
apps/web/src/app/(app)/standup/loading.tsxapps/web/src/app/(app)/standup/page.tsxapps/web/src/app/api/standup/board/route.tsapps/web/src/app/api/standups/[id]/advance/route.tsapps/web/src/app/api/standups/[id]/focus/route.tsapps/web/src/app/api/standups/[id]/route.tsapps/web/src/app/api/standups/[id]/start/route.tsapps/web/src/app/api/standups/[id]/turns/[turnId]/blockers/route.tsapps/web/src/app/api/standups/[id]/turns/[turnId]/route.tsapps/web/src/app/api/standups/blockers/[blockerId]/route.tsapps/web/src/app/api/standups/rotation/route.tsapps/web/src/app/api/standups/route.tsapps/web/src/features/standup/buckets.tsapps/web/src/features/standup/person-tiles.tsxapps/web/src/features/standup/person-work.tsxapps/web/src/features/standup/roster.tsxapps/web/src/features/standup/standup-board.tsxapps/web/src/features/standup/standup-clock.tsapps/web/src/features/standup/standup-page.tsxapps/web/src/features/standup/standup-room.tsxapps/web/src/features/standup/standup-skeleton.tsxapps/web/src/features/standup/standup-timer.tsxapps/web/src/features/standup/turn-panel.tsxapps/web/src/lib/query/issue-search.tsapps/web/src/lib/query/keys.tsapps/web/src/lib/query/schemas.tsapps/web/src/lib/query/use-issues.tsapps/web/src/lib/query/use-standup-board.tsapps/web/src/lib/query/use-standup.tsapps/web/src/lib/realtime/delta-bridge.tsxapps/web/tests/app/api/standup-board.test.tsapps/web/tests/features/standup/buckets.test.tsapps/web/tests/features/standup/person-tiles.test.tsxapps/web/tests/features/standup/person-work.test.tsxapps/web/tests/features/standup/standup-board.test.tsxapps/web/tests/features/standup/standup-clock.test.tsapps/web/tests/lib/query/use-standup-board.test.tsxapps/web/tests/lib/realtime/delta-bridge.test.tsxpackages/core/src/index.tspackages/core/src/work/issue-service.tspackages/core/src/work/standup-board.tspackages/core/tests/work/standup-board.test.tspackages/shared/src/validators/standup.ts
💤 Files with no reviewable changes (16)
- apps/web/src/lib/query/issue-search.ts
- apps/web/src/app/api/standups/blockers/[blockerId]/route.ts
- apps/web/src/app/api/standups/[id]/turns/[turnId]/blockers/route.ts
- apps/web/src/app/api/standups/rotation/route.ts
- apps/web/src/app/api/standups/route.ts
- apps/web/src/features/standup/standup-page.tsx
- apps/web/src/app/api/standups/[id]/start/route.ts
- apps/web/src/app/api/standups/[id]/advance/route.ts
- apps/web/src/app/api/standups/[id]/route.ts
- apps/web/src/features/standup/standup-room.tsx
- apps/web/src/app/api/standups/[id]/focus/route.ts
- apps/web/src/features/standup/roster.tsx
- apps/web/src/features/standup/turn-panel.tsx
- apps/web/src/lib/query/use-issues.ts
- apps/web/src/lib/query/use-standup.ts
- apps/web/src/app/api/standups/[id]/turns/[turnId]/route.ts
The route passed the raw query string to the service, so it never validated its own boundary. Its test only passed because the mock did the parsing, which made the test vacuous: removing the validation left it green. The route parses with the shared schema now, the mock records the input untouched, and four of the six cases fail if the parse is removed. The selection stranded on a member who left the roster while the board was open: the position read zero of N and the panel said Unknown member until somebody pressed an arrow. The active person falls back to the first tile as soon as the selected one is gone. The timer measured the age of the page rather than the length of the turn, so moving to the next person carried the previous person's elapsed time. It restarts on every move, by click or by arrow. It also stayed reachable while the peek was open, so t typed over an issue toggled it; it is gated the same way the arrow keys already were. A capped column header read 25 showing 2 of 25, printing the same total twice. It shows the bare count when nothing is hidden and the showing form when something is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
imshashank has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
1 participant
Closes the standup complaint directly: "it's very poorly designed... How will we actually run stand-up? It's super slow. Every click is super slow... it says, first of all, about the wrong project. It should be for the organization in the workspace, not for a given project... I'm not able to click on anybody's tasks at all."
What it is now
A row of person tiles across the top for everybody in the workspace. Click a tile, or press left and right, and that person's work appears in three columns: closed since the last standup, in progress, up next. Issues are real rows that open the peek. No attendance step, no facilitator, no turn, no required notes. A timer for anyone who wants to keep the round moving.
Why it is fast
Measured against the running dev server with a real browser:
Selection is pure local state. The old room wrote to the database on every click and then forced an invalidate, so each step cost two round trips and only ever showed one team.
Server
One windowed query over issues, organisation scoped with the viewer's team filter, whose universe is "still open, or closed since the window start". Capped per person per column via
row_number() over (partition by assignee_id, bucket), so a burst of fresh backlog cannot evict somebody's stale in-progress work. Exact counts come from a separate grouped query, so a capped column saysshowing 25 of 41rather than quietly under-reporting. The window start is the viewer's previous working day, Zod validated and clamped server side to 14 days.What was removed
The ceremony REST routes (
/api/standups/**) and the room components.packages/core/src/work/standup-service.tsand the four scrum tables stay untouched, because the MCP scrum tools import that service directly rather than over HTTP. Every deletion was grepped for importers first.Two things worth flagging
A judgement call. Two corrections from the design review contradicted each other about the delta bridge. One said leave the
standupmodel branch invalidating the board; the other said make it a no-op, since the board reads only issues and never the scrum tables. I took the no-op and moved the flag to theissuebranch, so an MCP-only ceremony write no longer refetches a board that cannot have changed.No Playwright spec. The e2e job is a CI gate and an unverified spec is more likely to turn it red than to catch anything. The one-request assertion lives in
apps/web/tests/features/standup/standup-board.test.tsxwithfetchstubbed, and I confirmed the behaviour by hand against a real browser as shown above. The old/standuphad no e2e coverage either, so this is not a regression.bun run verifyis green: 1724 tests across 9 packages, 0 failures.