Crew Sync v2 Phase 5e: coverage convention + guardrail - #533
Merged
Conversation
Adds the CLAUDE.md convention and unit/guardrails/crew-sync-coverage.test.ts per docs/CREW_SYNC_V2_PHASES.md section 5e: every Supabase-backed table the crew PWA caches in Dexie must be classified in lib/dexie/schema.ts's new CREW_SYNCED_TABLES/LOCAL_ONLY_TABLES exports, and (if synced) covered by either a broadcast trigger or the guardrail's SAFETY_POLL_ONLY allowlist — a new cached table now fails CI until it's consciously placed in one of these sets. Classified the current schema: turnovers/checklist_instances/ checklist_instance_items/crew_work_orders(->work_orders) are covered by the Phase 2 broadcast triggers; inventory_items/properties (pulled inside the turnovers scope pull, not their own trigger)/crew_availability/ messages/property_assets rely on the safety poll only. Also records the Realtime concurrent-clients quota check from docs/CREW_SYNC_V2_PHASES.md section 5a (10,000 max vs ~1,500 crew-fleet target — comfortable headroom) and the accompanying database connection pool size bump (2 -> 15, since Phase 3's all-private-channel design routes every join/reconnect's RLS authorization through that pool). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WboJd7a4GYvZK5BedZC2M6
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Contributor
📝 WalkthroughWalkthroughThe PR documents the Crew Sync v2 coverage convention, exports Dexie table classifications, and adds a guardrail test validating Supabase triggers, safety-poll allowlists, and complete ChangesCrew Sync v2 coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant crew-sync-coverage
participant FieldStayDexie
participant SupabaseMigration
crew-sync-coverage->>SupabaseMigration: Read broadcast-trigger migration SQL
crew-sync-coverage->>SupabaseMigration: Check TRIGGERED_TABLES coverage
crew-sync-coverage->>FieldStayDexie: Parse declared Dexie tables
crew-sync-coverage->>crew-sync-coverage: Check synced and local-only classifications
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
unit/guardrails/crew-sync-coverage.test.ts (2)
42-50: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRead all matching broadcast-trigger migrations.
Line 43 selects only one file. Split or follow-up
*crew_sync_broadcast*.sqlmigrations will be validated incorrectly. Aggregate every matching SQL file before checking triggers.Proposed fix
-function findBroadcastMigrationSql(): string { - const file = readdirSync(MIGRATIONS_DIR).find((f) => f.includes('crew_sync_broadcast')) - if (!file) { +function readBroadcastMigrationSql(): string { + const files = readdirSync(MIGRATIONS_DIR).filter( + (file) => file.endsWith('.sql') && file.includes('crew_sync_broadcast'), + ) + if (files.length === 0) { throw new Error( 'No supabase/migrations/*crew_sync_broadcast*.sql file found — did the ' + 'Crew Sync v2 Phase 2 migration get renamed, moved, or deleted?' ) } - return readFileSync(join(MIGRATIONS_DIR, file), 'utf8') + return files.map((file) => readFileSync(join(MIGRATIONS_DIR, file), 'utf8')).join('\n') }🤖 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 `@unit/guardrails/crew-sync-coverage.test.ts` around lines 42 - 50, Update findBroadcastMigrationSql to collect every migration filename matching crew_sync_broadcast rather than selecting only the first result. Read each matching file from MIGRATIONS_DIR and combine their SQL contents into one string, while preserving the existing error when no matching files are found.
79-90: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueParse the full Dexie class for
Tablefields.The current substring stops at the constructor and only matches
name!: Table<...>, so valid DexieTablefields declared after the constructor, or under modifiers such asreadonly, would be omitted from this guardrail. Parse all property declarations forFieldStayDexieinstead, for example at the dexie schema class definition.🤖 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 `@unit/guardrails/crew-sync-coverage.test.ts` around lines 79 - 90, Update the guardrail around class FieldStayDexie to inspect the entire class body rather than stopping at constructor(userId: string). Expand the Table field extraction to recognize valid property declarations regardless of modifiers such as readonly, while retaining the zero-match sanity check and declared table-name collection.
🤖 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.
Nitpick comments:
In `@unit/guardrails/crew-sync-coverage.test.ts`:
- Around line 42-50: Update findBroadcastMigrationSql to collect every migration
filename matching crew_sync_broadcast rather than selecting only the first
result. Read each matching file from MIGRATIONS_DIR and combine their SQL
contents into one string, while preserving the existing error when no matching
files are found.
- Around line 79-90: Update the guardrail around class FieldStayDexie to inspect
the entire class body rather than stopping at constructor(userId: string).
Expand the Table field extraction to recognize valid property declarations
regardless of modifiers such as readonly, while retaining the zero-match sanity
check and declared table-name collection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b092d82e-7fd6-466f-b879-0fa2c6a19191
📒 Files selected for processing (4)
CLAUDE.mddocs/CREW_SYNC_V2_PHASES.mdlib/dexie/schema.tsunit/guardrails/crew-sync-coverage.test.ts
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
2 participants
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.
Adds the CLAUDE.md convention and unit/guardrails/crew-sync-coverage.test.ts per docs/CREW_SYNC_V2_PHASES.md section 5e: every Supabase-backed table the crew PWA caches in Dexie must be classified in lib/dexie/schema.ts's new CREW_SYNCED_TABLES/LOCAL_ONLY_TABLES exports, and (if synced) covered by either a broadcast trigger or the guardrail's SAFETY_POLL_ONLY allowlist — a new cached table now fails CI until it's consciously placed in one of these sets.
Classified the current schema: turnovers/checklist_instances/ checklist_instance_items/crew_work_orders(->work_orders) are covered by the Phase 2 broadcast triggers; inventory_items/properties (pulled inside the turnovers scope pull, not their own trigger)/crew_availability/ messages/property_assets rely on the safety poll only.
Also records the Realtime concurrent-clients quota check from docs/CREW_SYNC_V2_PHASES.md section 5a (10,000 max vs ~1,500 crew-fleet target — comfortable headroom) and the accompanying database connection pool size bump (2 -> 15, since Phase 3's all-private-channel design routes every join/reconnect's RLS authorization through that pool).
Claude-Session: https://claude.ai/code/session_01WboJd7a4GYvZK5BedZC2M6
Summary by CodeRabbit
Documentation
Tests