Skip to content

Board/Calendar: preserve groupBy on fresh shard connect (closes #217) #263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brylie merged 1 commit into from
Sep 12, 2026
Merged

Board/Calendar: preserve groupBy on fresh shard connect (closes #217) #263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/lib/components/BoardCollectionView.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@
});

function handleSnapshot(snapshot: CollectionViewSnapshot): void {
// A fresh shard connection emits an initial empty snapshot before the
// WebSocket sync completes (snapshot.collection is undefined) — running
// autoPickGroupBy or announcer.notify against that empty doc would wipe
// an already-persisted groupBy and falsely baseline row diffs (issue #217).
if (!snapshot.collection) return;

@qodo-code-review qodo-code-review Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Fresh-connect behavior is undocumented 📘 Rule violation § Compliance

handleSnapshot and Table's snapshot callback now ignore snapshots where snapshot.collection is
absent, changing when grouping and remote-update baselines initialize. On a fresh or reconnecting
shard this defers initialization until synced metadata arrives across Board, Calendar, and Table,
but the collection-view and collaboration specifications do not define that transition.
Agent Prompt 
## Issue description
Board, Calendar, and Table now ignore the initial collection-less snapshot emitted before shard synchronization, but the corresponding specifications do not document when grouping and announcement baselines initialize.

## Fix Focus Areas
- docs/specifications/collection-views.md[32-34]
- docs/specifications/collaboration.md[19-26]

## Recommended Fix
Update the collection-view specification to state that snapshot consumers defer grouping initialization until collection metadata is available. Update the collaboration specification to clarify that collection-less pre-sync snapshots do not seed the remote-update baseline and that the first populated snapshot does.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@qodo-code-review qodo-code-review Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Screen readers retain stale updates 🐞 Bug ≡ Correctness

handleSnapshot and Table's snapshot callback return before announcer.notify whenever collection
metadata is absent, so the announcer cannot clear its current text or reset its row baseline. When
an observed collection is deleted or a retargeted shard never yields metadata, Board, Calendar, and
Table empty their rendered data but leave the previous remote-update status and baseline attached to
the now-missing view.
Agent Prompt 
## Issue description
Collection-less snapshots must not be diffed as empty collections, but returning immediately preserves stale live-region text and the previous collection baseline when metadata genuinely disappears.

## Fix Focus Areas
- src/lib/components/BoardCollectionView.svelte[93-98]
- src/lib/components/CalendarCollectionView.svelte[88-93]
- src/lib/components/TableCollectionView.svelte[93-96]
- src/lib/client/collection-announcer.svelte.ts[125-140]

## Recommended Fix
Add an explicit announcer reset operation that clears text, baseline, collection identity, toggling state, and pending local removals without diffing rows. Invoke that reset for collection-less snapshots in all three views, then return before auto-grouping or external snapshot processing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

announcer.notify(snapshot.collectionId, snapshot.rows);
if (autoGroupByAttempted) return;
autoGroupByAttempted = true;
Expand Down
23 changes: 23 additions & 0 deletions src/lib/components/BoardCollectionView.svelte.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,29 @@ describe('BoardCollectionView', () => {
expect(screen.getByText('No Status')).toBeInTheDocument();
});

it('preserves persisted groupBy and renders columns when collection arrives after initial mount (issue #217)', async () => {
const onConfigChange = vi.fn();
renderBoard('col-1', { sort: { mode: 'manual' }, groupBy: 'status' }, onConfigChange);

createCollection(ydoc, {
id: 'col-1',
title: 'Board',
schema: [
{
key: 'status',
label: 'Status',
type: 'select',
options: [{ id: 'todo', label: 'To do' }]
}
]
});

expect(await screen.findByText('To do')).toBeInTheDocument();
expect(onConfigChange).not.toHaveBeenCalledWith(
expect.objectContaining({ groupBy: undefined })
);
});

it('makes the card title directly editable via its own field', async () => {
createCollection(ydoc, {
id: 'col-1',
Expand Down
5 changes: 5 additions & 0 deletions src/lib/components/CalendarCollectionView.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@
});

function handleSnapshot(snapshot: CollectionViewSnapshot): void {
// A fresh shard connection emits an initial empty snapshot before the
// WebSocket sync completes (snapshot.collection is undefined) — running
// autoPickGroupBy or announcer.notify against that empty doc would wipe
// an already-persisted date groupBy and falsely baseline event diffs (issue #217).
if (!snapshot.collection) return;
announcer.notify(snapshot.collectionId, snapshot.rows);
if (autoGroupByAttempted) return;
autoGroupByAttempted = true;
Expand Down
27 changes: 25 additions & 2 deletions src/lib/components/CalendarCollectionView.svelte.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ vi.mock('$lib/client/yjs-client', () => ({

const actor = { kind: 'human' as const, userId: 'local' };

function renderCalendar(collectionId: string, initialConfig: ViewConfig = {}) {
return render(CalendarCollectionViewHarness, { collectionId, initialConfig });
function renderCalendar(
collectionId: string,
initialConfig: ViewConfig = {},
onConfigChange?: (config: ViewConfig) => void
) {
return render(CalendarCollectionViewHarness, { collectionId, initialConfig, onConfigChange });
}

describe('CalendarCollectionView', () => {
Expand Down Expand Up @@ -68,6 +72,25 @@ describe('CalendarCollectionView', () => {
]);
});

it('preserves persisted groupBy when collection arrives after initial mount (issue #217)', async () => {
const onConfigChange = vi.fn();
renderCalendar('col-1', { groupBy: 'due' }, onConfigChange);

createCollection(ydoc, {
id: 'col-1',
title: 'Cal',
schema: [
{ key: 'title', label: 'Title', type: 'text' },
{ key: 'due', label: 'Due', type: 'date' }
]
});

expect(await screen.findByRole('option', { name: 'Due' })).toBeInTheDocument();
expect(onConfigChange).not.toHaveBeenCalledWith(
expect.objectContaining({ groupBy: undefined })
);
});

it('places a record on its matching day cell', async () => {
createCollection(ydoc, {
id: 'col-1',
Expand Down
1 change: 1 addition & 0 deletions src/lib/components/TableCollectionView.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
() => ydoc,
() => connection.resolvedCollectionId ?? collectionId,
(snapshot) => {
if (!snapshot.collection) return;
announcer.notify(snapshot.collectionId, snapshot.rows);
onSnapshot?.(snapshot);
}
Expand Down
144 changes: 143 additions & 1 deletion tests/e2e/tier-b.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { test, expect } from '@playwright/test';
import { createTestHarness, type TestHarness } from './harness';
import { createDocument, createRecord } from '$lib/services';
import { createCollection, createDocument, createRecord } from '$lib/services';
import { flush, resolveWorkspaceContext } from '$lib/server/workspace-store';
import type { ActorId } from '$lib/data/types';

Expand Down Expand Up @@ -301,4 +301,146 @@ test.describe('Tier B: DOM-visible MCP/Browser parity', () => {
const response = await page.request.get(new URL(faviconHref!, page.url()).toString());
expect(response.status()).toBe(200);
});

test('Board view groupBy columns render on initial load after fresh connect (issue #217)', async ({
page
}) => {
const collection = createCollection(human, {
title: 'Sprint Tasks',
schema: [
{
key: 'status',
label: 'Status',
type: 'select',
options: [
{ id: 'todo', label: 'To Do' },
{ id: 'done', label: 'Done' }
]
}
]
});
createRecord(human, {
parentId: collection.id,
properties: {
status: { type: 'select', value: 'todo' }
}
});

const docMeta = createDocument(human, {
title: 'Board Page',
createInitialBlock: false
});
createRecord(human, {
parentId: docMeta.id,
blockType: 'collection_view',
referencedRecordId: collection.id,
viewConfig: {
viewType: 'board',
groupBy: 'status'
}
});
flush();

// Open document fresh in browser
await page.goto(`${harness.httpUrl}/space/${defaultSpaceId()}/doc/${docMeta.id}`);

// Columns should render!
await expect(page.getByRole('group', { name: 'To Do column' })).toBeVisible();
await expect(page.getByRole('group', { name: 'Done column' })).toBeVisible();
await expect(page.getByRole('group', { name: 'No Status column' })).toBeVisible();
// No unsaved changes banner
await expect(page.getByText('Unsaved changes')).not.toBeVisible();
});

test('Board view auto-picks groupBy on initial load when unset (issue #217)', async ({
page
}) => {
const collection = createCollection(human, {
title: 'Sprint Tasks Auto',
schema: [
{
key: 'status',
label: 'Status',
type: 'select',
options: [
{ id: 'todo', label: 'To Do' },
{ id: 'done', label: 'Done' }
]
}
]
});
createRecord(human, {
parentId: collection.id,
properties: {
status: { type: 'select', value: 'todo' }
}
});

const docMeta = createDocument(human, {
title: 'Board Page Auto',
createInitialBlock: false
});
createRecord(human, {
parentId: docMeta.id,
blockType: 'collection_view',
referencedRecordId: collection.id,
viewConfig: {
viewType: 'board'
}
});
flush();

// Open document fresh in browser
await page.goto(`${harness.httpUrl}/space/${defaultSpaceId()}/doc/${docMeta.id}`);

// Columns should render automatically via autoPickGroupBy!
await expect(page.getByRole('group', { name: 'To Do column' })).toBeVisible();
await expect(page.getByRole('group', { name: 'Done column' })).toBeVisible();
await expect(page.getByRole('group', { name: 'No Status column' })).toBeVisible();
});

test('Calendar view preserves persisted date groupBy on fresh connect (issue #217)', async ({
page
}) => {
const collection = createCollection(human, {
title: 'Sprint Milestones',
schema: [
{
key: 'due',
label: 'Due Date',
type: 'date'
}
]
});
createRecord(human, {
parentId: collection.id,
properties: {
due: { type: 'date', value: '2026-09-15' }
}
});

const docMeta = createDocument(human, {
title: 'Calendar Page',
createInitialBlock: false
});
createRecord(human, {
parentId: docMeta.id,
blockType: 'collection_view',
referencedRecordId: collection.id,
viewConfig: {
viewType: 'calendar',
groupBy: 'due'
}
});
flush();

// Open document fresh in browser
await page.goto(`${harness.httpUrl}/space/${defaultSpaceId()}/doc/${docMeta.id}`);

// "Dates from" dropdown should show "Due Date" and no unsaved changes
const select = page.locator(`select[id="calendar-date-property-${collection.id}"]`);
await expect(select).toBeVisible();
await expect(select).toHaveValue('due');
await expect(page.getByText('Unsaved changes')).not.toBeVisible();
});
});
Loading