Board/Calendar: preserve groupBy on fresh shard connect (closes #217) - #263
Merged
Conversation
When a CollectionView block connects to its shard, useCollectionView initially emits a synchronous snapshot before the WebSocket provider has synced with the server, meaning snapshot.collection is undefined and snapshot.schema/rows are empty. In BoardCollectionView and CalendarCollectionView, handleSnapshot was marking autoGroupByAttempted = true and executing autoPickGroupBy against that empty schema. Because the schema was empty, autoPickGroupBy failed to resolve the persisted groupBy property and reset it to undefined, marking the draft dirty and causing Board columns and Calendar dates to render completely empty. A subsequent real snapshot after sync was ignored because autoGroupByAttempted had already been set. In addition, announcer.notify was prematurely baselining against an empty list, erroneously announcing existing records as newly added. Guards handleSnapshot (and TableCollectionView's onSnapshot handler) to return early if snapshot.collection is not yet populated. Adds unit and Tier B Playwright regression coverage.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (6)
Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthroughCollection views now ignore initial snapshots without a collection. Board and Calendar views preserve persisted grouping, while Table avoids premature callbacks. Component and end-to-end tests cover delayed collection initialization and fresh-load grouping behavior. ChangesCollection snapshot synchronization
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The fix preserves persisted grouping during initial synchronization and is covered by unit and end-to-end tests. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoPreserve collection grouping across fresh shard connections
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
1. Screen readers retain stale updates
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| // 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; |
There was a problem hiding this comment.
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
| // 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; |
There was a problem hiding this comment.
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #217: on fresh page load or reconnect, Board and Calendar view column/date grouping rendered completely empty even when
config.groupBywas already valid and persisted.Root Cause
When a
collection_viewblock connects to its shard:useCollectionConnectioninitializes a newY.DocandWebsocketProvider.useCollectionViewobserves this doc and immediately triggers a synchronousrefresh().doc.getMap('collections')is empty (snapshot.collection === undefined,snapshot.schema === []).BoardCollectionViewandCalendarCollectionView,handleSnapshotwas running without checkingsnapshot.collection:autoGroupByAttempted = true.autoPickGroupByagainst the empty schema[].autoPickGroupByfailed to findconfig.groupByin the empty schema, resolved it toundefined, and calledonConfigChange({ ...config, groupBy: undefined }).groupBysetting, marked the draft dirty ("Unsaved changes"), and caused columns/dates to render empty (columns = []).autoGroupByAttemptedwas alreadytrue, sohandleSnapshotreturned early without restoring the grouping property.announcer.notifyprematurely seeded its baseline againstrows: [], causing existing records to be erroneously announced as newly added by remote collaborators.Solution
BoardCollectionView.svelte: GuardhandleSnapshotwithif (!snapshot.collection) return;.CalendarCollectionView.svelte: GuardhandleSnapshotwithif (!snapshot.collection) return;.TableCollectionView.svelte: GuardonSnapshotcallback withif (!snapshot.collection) return;before callingannouncer.notify.tests/e2e/tier-b.spec.tsfor Board fresh connect with persisted groupBy, Board auto-pick when unset, and Calendar fresh connect with persisted groupBy.BoardCollectionView.svelte.test.tsandCalendarCollectionView.svelte.test.tsverifying that when collection metadata arrives after initial mount, persisted groupBy is preserved.Verification
npm run test:unit: All 1,279 unit tests pass.npm run test:e2e:tier-b: All 9 Tier B Playwright tests pass.npm run check: 0 errors, 0 warnings.npm run lint: All checks pass.scripts/pre-push-check.sh: Passed.Summary by CodeRabbit
Bug Fixes
Tests