FBOSS Review
Overview
Multi-reviewer code review for FBOSS diffs. Subsumes general review (/review-diff functionality) plus FBOSS-specific domain expertise.
Scope
Currently fboss/ only. TODO: extend to configerator/source/neteng/fboss, neteng/netcastle, neteng/fboss.
IMPORTANT: No Auto-Posting
All findings are shown to the user only. Never post to Phabricator automatically.
Review Workflow
Step 1: Identify Changes
- Run
sl status and sl diff to get changed files
- If user specifies a diff number, fetch diff details via
mcp__plugin_meta_mux__get_phabricator_diff_details
Step 2: Determine Applicable FBOSS Reviewers
- Files in
agent/, SwSwitch, HwSwitch -> Agent Reviewer (#6)
- Files in
fsdb/, thrift_cow/ -> FSDB/thrift_cow Reviewer (#7)
- Files in
platform/, config, sensor/fan -> Platform Reviewer (#8)
- Files in
sai/, hw/sai/, SDK -> SAI/SDK Reviewer (#9)
- Test files or coverage-affecting changes -> Testing Reviewer (#10)
- Cross-cutting changes (SwSwitch + HwSwitch, state, counters, build targets, symbol exports, client infrastructure, or 3+ directories) -> Architect Reviewer (#11)
Step 3: Dispatch Reviewers in Parallel
Use Agent tool (sonnet model) to dispatch all applicable reviewers simultaneously:
- Generic reviewers 1-5: ALWAYS dispatch. Use personas from
references/generic-reviewers.md.
- FBOSS reviewers 6-11: Dispatch ONLY if their area is touched.
- Each reviewer receives: full diff content + their persona instructions.
- FBOSS reviewers additionally load patterns from
../fboss-code-standards/references/<area>-patterns.md.
| # |
Reviewer |
Focus |
Patterns File |
| 1 |
Reliability |
Error handling, RAII, logging, timeout/retry, graceful degradation |
- |
| 2 |
Engineering / Performance |
Algorithmic complexity, unnecessary copies, lock contention, modern C++ |
- |
| 3 |
Code Quality |
Readability, modularity, duplication, API design |
../fboss-code-standards/references/general-patterns.md |
| 4 |
Summary & Test Plan |
Title accuracy, summary completeness, test plan adequacy, diff coherence |
- |
| 5 |
Silent Failure Finder |
Logic errors, lossy conversions, race conditions, silent data loss |
- |
| 6 |
Agent Architecture |
Mono/multi-switch, state management, warmboot, HwSwitch/SwSwitch boundary |
../fboss-code-standards/references/agent-patterns.md |
| 7 |
FSDB & thrift_cow |
State/Stats duality, COW node safety, subscriptions, build time |
../fboss-code-standards/references/thrift-cow-fsdb-patterns.md |
| 8 |
Platform & Config |
Platform services, JSON configs, startup order, OSS sync |
../fboss-code-standards/references/platform-config-patterns.md |
| 9 |
SAI/SDK Integration |
SAI API usage, object lifecycle, vendor SDK compat, attributes |
../fboss-code-standards/references/sai-sdk-patterns.md |
| 10 |
Testing Standards |
Coverage, naming, fixtures, NSDB impact, HW test patterns |
../fboss-code-standards/references/testing-patterns.md |
| 11 |
FBOSS Architect |
Cross-cutting design: layering, state derivability, abstraction, code org, infra reuse |
references/fboss-specific-reviewers.md |
Step 4: Verification
Dispatch a single verifier agent (loads references/verifier.md) to:
- Deduplicate findings across reviewers
- Calibrate confidence scores
- Filter false positives
- Threshold: only report findings with confidence >= 0.7
Step 5: Present Findings
Output a structured report:
| File:Line | Reviewer | Severity | Issue | Confidence |
|-----------|----------|----------|-------|------------|
Never post findings to Phabricator.
When to Load References
| Topic |
Action |
| Generic reviewer personas |
Read references/generic-reviewers.md |
| Verification/dedup logic |
Read references/verifier.md |
| FBOSS-specific reviewer personas |
Read references/fboss-specific-reviewers.md |
| Adding/modifying reviewers |
Read references/contributing.md |
1---2name: fboss-review3description: Comprehensive FBOSS code review with 11 parallel reviewers (5 generic + 6 FBOSS-specific). Covers reliability, engineering, code quality, silent failures, agent architecture, SAI/SDK, FSDB/thrift_cow, platform/config, testing, and cross-cutting design. Self-contained alternative to /review-diff for FBOSS diffs. Findings shown to user only, never auto-posted.4---56# FBOSS Review78## Overview910Multi-reviewer code review for FBOSS diffs. Subsumes general review (`/review-diff` functionality) plus FBOSS-specific domain expertise.1112## Scope1314Currently `fboss/` only. TODO: extend to `configerator/source/neteng/fboss`, `neteng/netcastle`, `neteng/fboss`.1516## IMPORTANT: No Auto-Posting1718All findings are shown to the user only. Never post to Phabricator automatically.1920## Review Workflow2122### Step 1: Identify Changes2324- Run `sl status` and `sl diff` to get changed files25- If user specifies a diff number, fetch diff details via `mcp__plugin_meta_mux__get_phabricator_diff_details`2627### Step 2: Determine Applicable FBOSS Reviewers2829- Files in `agent/`, `SwSwitch`, `HwSwitch` -> Agent Reviewer (#6)30- Files in `fsdb/`, `thrift_cow/` -> FSDB/thrift_cow Reviewer (#7)31- Files in `platform/`, config, sensor/fan -> Platform Reviewer (#8)32- Files in `sai/`, `hw/sai/`, SDK -> SAI/SDK Reviewer (#9)33- Test files or coverage-affecting changes -> Testing Reviewer (#10)34- Cross-cutting changes (SwSwitch + HwSwitch, state, counters, build targets, symbol exports, client infrastructure, or 3+ directories) -> Architect Reviewer (#11)3536### Step 3: Dispatch Reviewers in Parallel3738Use Agent tool (sonnet model) to dispatch all applicable reviewers simultaneously:3940- **Generic reviewers 1-5**: ALWAYS dispatch. Use personas from `references/generic-reviewers.md`.41- **FBOSS reviewers 6-11**: Dispatch ONLY if their area is touched.42- Each reviewer receives: full diff content + their persona instructions.43- FBOSS reviewers additionally load patterns from `../fboss-code-standards/references/<area>-patterns.md`.4445| # | Reviewer | Focus | Patterns File |46|---|----------|-------|---------------|47| 1 | Reliability | Error handling, RAII, logging, timeout/retry, graceful degradation | - |48| 2 | Engineering / Performance | Algorithmic complexity, unnecessary copies, lock contention, modern C++ | - |49| 3 | Code Quality | Readability, modularity, duplication, API design | `../fboss-code-standards/references/general-patterns.md` |50| 4 | Summary & Test Plan | Title accuracy, summary completeness, test plan adequacy, diff coherence | - |51| 5 | Silent Failure Finder | Logic errors, lossy conversions, race conditions, silent data loss | - |52| 6 | Agent Architecture | Mono/multi-switch, state management, warmboot, HwSwitch/SwSwitch boundary | `../fboss-code-standards/references/agent-patterns.md` |53| 7 | FSDB & thrift_cow | State/Stats duality, COW node safety, subscriptions, build time | `../fboss-code-standards/references/thrift-cow-fsdb-patterns.md` |54| 8 | Platform & Config | Platform services, JSON configs, startup order, OSS sync | `../fboss-code-standards/references/platform-config-patterns.md` |55| 9 | SAI/SDK Integration | SAI API usage, object lifecycle, vendor SDK compat, attributes | `../fboss-code-standards/references/sai-sdk-patterns.md` |56| 10 | Testing Standards | Coverage, naming, fixtures, NSDB impact, HW test patterns | `../fboss-code-standards/references/testing-patterns.md` |57| 11 | FBOSS Architect | Cross-cutting design: layering, state derivability, abstraction, code org, infra reuse | `references/fboss-specific-reviewers.md` |5859### Step 4: Verification6061Dispatch a single verifier agent (loads `references/verifier.md`) to:62- Deduplicate findings across reviewers63- Calibrate confidence scores64- Filter false positives65- Threshold: only report findings with confidence >= 0.76667### Step 5: Present Findings6869Output a structured report:7071```72| File:Line | Reviewer | Severity | Issue | Confidence |73|-----------|----------|----------|-------|------------|74```7576Never post findings to Phabricator.7778## When to Load References7980| Topic | Action |81|-------|--------|82| Generic reviewer personas | Read `references/generic-reviewers.md` |83| Verification/dedup logic | Read `references/verifier.md` |84| FBOSS-specific reviewer personas | Read `references/fboss-specific-reviewers.md` |85| Adding/modifying reviewers | Read `references/contributing.md` |
facebook