--- description: Structured code review workflow for Verkada Backend PRs. Builds context from PR diff, Linear ticket, and vcerberus docs, then runs 8 parallel review tracks with sub-agents. category: verkada --- flowchart TD _HEADER_[""]:::headerStyle classDef headerStyle fill:none,stroke:none subgraph _MAIN_[" "] subgraph Context["Phase 1: Build Context"] START[Start Review] --> DIFF[Read PR Diff
gh pr diff] START --> COMMENTS[Read PR Comments
and Description] START --> LINEAR[Read Linear Ticket] START --> DOCS[Read vcerberus/docs
Related to PR Topic] DIFF --> CONTEXT_DONE[Context Built] COMMENTS --> CONTEXT_DONE LINEAR --> CONTEXT_DONE DOCS --> CONTEXT_DONE end subgraph Review["Phase 2: Parallel Review Tracks"] CONTEXT_DONE --> TESTING[Testing
Coverage + Style] CONTEXT_DONE --> INTEG[Integration Tests
Style Guide Compliance] CONTEXT_DONE --> MIGRATIONS[Database
Migrations] CONTEXT_DONE --> CLEAN[Cleanliness
Imports, Logs, Comments] CONTEXT_DONE --> FORGOT[Nothing Forgotten
TODOs, Docstrings, Etags] CONTEXT_DONE --> FORMAT[Formatting
Parameterized Logging] CONTEXT_DONE --> SA20[SQLAlchemy 2.0
Patterns] CONTEXT_DONE --> LOGIC[Logic Errors
Edge Cases, None Checks] end subgraph TestDetail["Testing Details"] TESTING --> TEST_COV{Test Coverage
Exists?} TEST_COV -->|Yes| TEST_TIGHT[Tests Tight
and Compact?] TEST_COV -->|No| TEST_MISSING[Flag: Missing
Test Coverage] TEST_TIGHT --> TEST_FILES{Unnecessary
New Test Files?} TEST_FILES -->|Yes| TEST_CONSOLIDATE[Flag: Consolidate
Into Existing File] TEST_FILES -->|No| TEST_OK[Tests OK] end subgraph IntegDetail["Integration Test Details"] INTEG --> INTEG_NEEDED{Integration
Tests Present?} INTEG_NEEDED -->|No| INTEG_SKIP[Skip] INTEG_NEEDED -->|Yes| INTEG_SETUP[Check setUp/tearDown
Calls super] INTEG_SETUP --> INTEG_ENTITIES[Universal Entities
in setUp Only] INTEG_ENTITIES --> INTEG_ENDPOINTS[Uses _vcerberus_server
Helper Methods with Docstrings] INTEG_ENDPOINTS --> INTEG_PERMS[Permission Tests
as_user + retry_session] INTEG_PERMS --> INTEG_MIXINS[Correct MixIns
and Inheritance] INTEG_MIXINS --> INTEG_OK[Integration OK] end subgraph DBDetail["Database Details"] MIGRATIONS --> MIG_PRESENT{Migration
Files?} MIG_PRESENT -->|No| MIG_SKIP[Skip] MIG_PRESENT -->|Yes| MIG_DOWN[Downgrade Reverts
Everything Including Indexes] MIG_DOWN --> MIG_DETACH[No Detached Objects
After Commits/Deletes] MIG_DETACH --> MIG_OK[Migrations OK] end subgraph SA2Detail["SA2.0 Details"] SA20 --> SA2_NEW{New Code
Only?} SA2_NEW -->|Existing| SA2_SKIP[Skip] SA2_NEW -->|New| SA2_SELECT[select + session.execute
Not session.query] SA2_SELECT --> SA2_MODEL[mapped_column + Mapped
Not bare Column] SA2_MODEL --> SA2_REL[back_populates
Not backref] SA2_REL --> SA2_LOADER[Class-bound Loader Options
Not Strings] SA2_LOADER --> SA2_OK[SA2.0 OK] end subgraph LogicDetail["Logic Details"] LOGIC --> LOGIC_ARRAY[Array Edge Cases] LOGIC_ARRAY --> LOGIC_NONE[is not None
vs Boolean Check] LOGIC_NONE --> LOGIC_SILENT[No Silent Failures
Empty Loops] LOGIC_SILENT --> LOGIC_SCHED{Schedule
Handling?} LOGIC_SCHED -->|No| LOGIC_OK[Logic OK] LOGIC_SCHED -->|Yes| LOGIC_SCHED_TYPES[All Schedule Types
door, user, exception, visit] LOGIC_SCHED_TYPES --> LOGIC_INHERIT[Access Level
Inheritance Respected] LOGIC_INHERIT --> LOGIC_OK end subgraph Outcome["Phase 3: Results"] TEST_OK --> COLLECT[Collect All
Review Findings] TEST_MISSING --> COLLECT TEST_CONSOLIDATE --> COLLECT INTEG_OK --> COLLECT INTEG_SKIP --> COLLECT MIG_OK --> COLLECT MIG_SKIP --> COLLECT CLEAN --> COLLECT FORGOT --> COLLECT FORMAT --> COLLECT SA2_OK --> COLLECT SA2_SKIP --> COLLECT LOGIC_OK --> COLLECT COLLECT --> SUMMARY[Review Summary
with Action Items] end click START "#" "**Start Review**\nBegin structured PR review.\nAll 4 context steps run in parallel." click DIFF "#" "**Read PR Diff**\n`gh pr diff --name-only` for file list\n`gh pr diff` for full changes\nIdentifies scope and affected areas." click COMMENTS "#" "**Read PR Comments**\nRead all inline comments, review threads,\nand the PR description.\nCaptures reviewer feedback and author intent." click LINEAR "#" "**Read Linear Ticket**\nUse `linear-cli` to fetch the associated ticket.\nUnderstand requirements, acceptance criteria,\nand design decisions." click DOCS "#" "**Read vcerberus/docs**\nFind and read docs related to the PR topic.\nExamples: permissions doc for RBAC changes,\nintegration-test-style-guide for test PRs,\napple-wallet.md for wallet changes." click CONTEXT_DONE "#" "**Context Built**\nAll 4 parallel context streams merged.\nReviewer now has full understanding of:\n- What changed (diff)\n- Why it changed (ticket + description)\n- How it should work (docs)\n- What others said (comments)" click TESTING "#" "**Testing Track**\nVerify test coverage exists for edited code.\nEnsure tests are tight, compact, not repetitive.\nCheck for unnecessary new test files --\nprefer adding to existing thematically-matching files." click INTEG "#" "**Integration Test Track**\nIf integration tests are present, verify they\nfollow vcerberus/docs/integration-test-style-guide.md" click MIGRATIONS "#" "**Database Migrations Track**\nCheck alembic migration files if present.\nVerify downgrade path and object lifecycle." click CLEAN "#" "**Cleanliness Track**\n- No temporary markdown notes in PR\n- Move inline imports to top of file\n- Comments only where code is not self-explanatory\n- Use logging, not print\n- logger.debug for generic debug (not logger.info)\n- Use shared common utils, no reinvention" click FORGOT "#" "**Nothing Forgotten Track**\n- No unresolved TODO: items\n- Docstrings updated for changed endpoints/functions\n- Fix spelling mistakes\n- Etag headers on etag'd endpoints\n- PR description and title up to date\n- Title starts with [ticket-id]" click FORMAT "#" "**Formatting Track**\nEnsure logger statements use parameterized logging:\n`logger.info('msg %s', var)` not `logger.info(f'msg {var}')`\nThis enables proper log aggregation in Datadog." click SA20 "#" "**SQLAlchemy 2.0 Track**\nNew code only -- do not flag existing patterns.\nReference: `.claude/rules/vcerberus/sqlalchemy.md`" click LOGIC "#" "**Logic Errors Track**\nCheck for common logic bugs:\n- Array edge cases (empty, single, boundary)\n- `is not None` vs plain boolean check\n- Silent failures (empty for-loops, swallowed exceptions)" click TEST_COV "#" "**Test Coverage Check**\nIs the code being changed covered by tests?\nBoth unit and integration tests count." click TEST_TIGHT "#" "**Tests Tight and Compact?**\nNo tests performing basically the same thing\nover and over. Each test should cover a distinct case." click TEST_MISSING "#" "**Missing Test Coverage**\nFlag that tests need to be added\nfor the changed code paths." click TEST_FILES "#" "**Unnecessary New Test Files?**\nIf a thematically-matching test file already exists,\nnew tests should be added there instead of\ncreating a new file." click TEST_CONSOLIDATE "#" "**Consolidate Tests**\nFlag: tests should be moved into\nan existing test file." click TEST_OK "#" "**Tests Pass Review**\nCoverage exists, tests are compact,\nno unnecessary files." click INTEG_NEEDED "#" "**Integration Tests Present?**\nCheck if the PR includes integration test changes.\nIf not, skip this track." click INTEG_SKIP "#" "**Skip Integration Review**\nNo integration tests in this PR." click INTEG_SETUP "#" "**setUp/tearDown Pattern**\nMust call `super().setUp()` and `super().tearDown()`.\nNo lazy loading patterns." click INTEG_ENTITIES "#" "**Entity Creation**\nUniversal entities (Controller, User, site) in setUp.\nTest-specific entities only in individual tests.\nFollow reserved terminology: `self.user`, `self.admin`,\n`self.organization_id`, etc." click INTEG_ENDPOINTS "#" "**Endpoint Access**\nUse `self._vcerberus_server` for URLs.\nNot `self.config.get('web_host')`.\nEndpoint calls via helper methods with docstrings,\nnot inline http calls." click INTEG_PERMS "#" "**Permission Testing**\nCreate users with `User()` + `created_logged_in_session(use_prev=True)`.\nUnified permission tests (not separate positive/negative).\nUse helpers with `as_user` parameter.\nAll sessions via `retry_session()`." click INTEG_MIXINS "#" "**Correct MixIns**\nInherit from `IntegrationTestBase, VCerberusIntegrationTestBase`.\nUse relevant MixIns: CreateUserGroupMixin,\nTestSchedulesMixin, etc." click INTEG_OK "#" "**Integration Tests Pass Review**\nAll patterns follow the style guide." click MIG_PRESENT "#" "**Migration Files Present?**\nCheck for alembic migration files in the PR diff." click MIG_SKIP "#" "**Skip Migration Review**\nNo migration files in this PR." click MIG_DOWN "#" "**Downgrade Path**\nThe downgrade function must revert everything\nthe upgrade creates, including indexes." click MIG_DETACH "#" "**No Detached Objects**\nEnsure no database objects become detached\nas a result of commits or deletes in the migration\nor related code." click MIG_OK "#" "**Migrations Pass Review**\nDowngrade is complete, no detachment risks." click SA2_NEW "#" "**New Code Only**\nSA2.0 patterns only apply to newly written code.\nDo not flag existing legacy patterns." click SA2_SKIP "#" "**Skip SA2.0 Review**\nNo new ORM code in this PR." click SA2_SELECT "#" "**select() Pattern**\nNew queries: `select()` + `session.execute()`\nNot `session.query()`.\nBulk ops: `update()`/`delete()` constructs.\nPK lookups: `session.get(Model, pk)`." click SA2_MODEL "#" "**Model Declarations**\n`mapped_column()` with `Mapped[]` type annotations.\nNot bare `Column()`.\nUse `DeclarativeBase`, not `declarative_base()`." click SA2_REL "#" "**Relationships**\nExplicit `back_populates=` on both sides.\nNot `backref=`." click SA2_LOADER "#" "**Loader Options**\nClass-bound attributes:\n`selectinload(User.addresses)`\nNot string-based: `selectinload('addresses')`.\nRaw SQL wrapped in `text()`." click SA2_OK "#" "**SA2.0 Patterns Pass Review**\nAll new code follows SQLAlchemy 2.0 style." click LOGIC_ARRAY "#" "**Array Edge Cases**\nCheck empty arrays, single-element arrays,\nand boundary conditions in loops and indexing." click LOGIC_NONE "#" "**None Checks**\nUse `is not None` / `is None` for None comparisons.\nNot plain boolean checks that conflate None with\nFalse, 0, or empty string." click LOGIC_SILENT "#" "**Silent Failures**\nNo empty for-loops that silently do nothing.\nNo swallowed exceptions without logging." click LOGIC_SCHED "#" "**Schedule Handling?**\nDoes this PR touch schedule-related code?" click LOGIC_SCHED_TYPES "#" "**All Schedule Types**\nEnsure all types are handled:\ndoor, user, exception, visit, etc.\nMissing a type is a common source of bugs." click LOGIC_INHERIT "#" "**Access Level Inheritance**\nSchedule inheritance via parent relationships\nmust be respected. Check access level\nschedule parent chains." click LOGIC_OK "#" "**Logic Pass Review**\nNo edge case issues found." click COLLECT "#" "**Collect Findings**\nAll 8 parallel review tracks report back.\nFindings categorized as: must-fix, should-fix, nit." click SUMMARY "#" "**Review Summary**\nStructured summary with:\n- Blocking issues (must-fix before merge)\n- Suggestions (should-fix, not blocking)\n- Nits (style, optional)\nPosted as PR review or reported to user." classDef context fill:#d1ecf1,stroke:#7ec8d8 classDef track fill:#e8daef,stroke:#b07cc6 classDef detail fill:#ffeaa7,stroke:#e0c040 classDef decision fill:#fff3cd,stroke:#f0c040 classDef success fill:#d4edda,stroke:#5cb85c classDef failure fill:#f8d7da,stroke:#e06070 classDef skip fill:#e2e8f0,stroke:#94a3b8 class START,DIFF,COMMENTS,LINEAR,DOCS,CONTEXT_DONE context class TESTING,INTEG,MIGRATIONS,CLEAN,FORGOT,FORMAT,SA20,LOGIC track class TEST_TIGHT,TEST_FILES,INTEG_SETUP,INTEG_ENTITIES,INTEG_ENDPOINTS,INTEG_PERMS,INTEG_MIXINS,MIG_DOWN,MIG_DETACH,SA2_SELECT,SA2_MODEL,SA2_REL,SA2_LOADER,LOGIC_ARRAY,LOGIC_NONE,LOGIC_SILENT,LOGIC_SCHED_TYPES,LOGIC_INHERIT detail class TEST_COV,INTEG_NEEDED,MIG_PRESENT,SA2_NEW,LOGIC_SCHED decision class TEST_OK,INTEG_OK,MIG_OK,SA2_OK,LOGIC_OK,COLLECT,SUMMARY success class TEST_MISSING,TEST_CONSOLIDATE failure class INTEG_SKIP,MIG_SKIP,SA2_SKIP skip end style _MAIN_ fill:none,stroke:none,padding:0 _HEADER_ ~~~ _MAIN_