Code review rubric (HyperbyteDB monorepo)¶

Authoritative checklist for reviewing changes in this repository. Each layer lists must-pass gates, should-fix items, and nice-to-have improvements. A gate passes only with cited evidence (command output, test name, or code trace).

How to use¶

1. Pick the layers that apply (Rust-only change vs. operator vs. CI).
2. For each section in section-review-notes.md, record findings with severity: blocker, major, minor, nit.
3. For removals (deps, files, tests), use Layer E first—no deletion without proof.

Tool gates (default PR)¶

These align with .github/workflows/ci.yml.

Container (when changing Dockerfile or image entrypoints): .github/workflows/container.yml on main / PRs.

Layer A — Rust library and service correctness¶

A.1 Ownership, errors, memory, API, async (baseline)¶

Use the project rust-skills rule set (summarized in .cursorrules): prioritize own-*, err-*, mem-*, api-*, async-*, opt-*, perf-*, anti-* as relevant to the diff.

Must-pass

• No new unwrap/expect on fallible paths in non-test code unless justified (startup invariant or test); production code already uses #![cfg_attr(not(test), warn(clippy::unwrap_used, clippy::expect_used))] on crate roots.
• Errors propagate with ? or explicit mapping; library boundaries use typed errors ([src/error.rs](../../src/error.rs)).
• No Mutex/RwLock held across .await without audit.
• Public API changes are intentional (visibility, serde stability).

Should-fix

• Prefer &str / &[T] at boundaries per rust-skills.
• Hot paths: avoid unnecessary clone/format! without profiling note.

A.2 Data path integrity (TSDB)¶

Scope: WAL → metadata → flush → Parquet layout; restart behavior.

Must-pass

• WAL ordering and sequence semantics preserved; flush does not drop acknowledged writes.
• Replication apply path is idempotent where the protocol requires it ([src/application/replication_apply.rs](../../src/application/replication_apply.rs)).
• Storage layout and file naming match [src/domain/storage_layout.rs](../../src/domain/storage_layout.rs) and adapters.

Evidence: targeted tests under tests/; trace through WalPort / StoragePort impls for the change.

A.3 Distributed semantics (cluster)¶

Scope: Raft, peer sync, hinted handoff, drain, membership.

Must-pass

• Network failures: timeouts, retries, and backoff match config; no unbounded queues without documentation.
• Raft log/store/network/state machine changes preserve safety (no committed log loss) per design in [docs/deep-dive/deep-dive-clustering.md](../deep-dive/deep-dive-clustering.md), [docs/developer-guide/internals/replication-design.md](../developer-guide/internals/replication-design.md).

Evidence: tests/e2e or tests/raft_integration.rs where applicable; code review of openraft TypeConfig and log store.

A.4 HTTP / InfluxDB v1 compatibility¶

Scope: [src/adapters/http/](../../src/adapters/http/).

Must-pass

• Status codes and bodies match expectations in [docs/user-guide/reference.md](../user-guide/reference.md#influxdb-v1-compatibility-matrix) (InfluxDB v1 surface) for touched endpoints.
• Auth and anonymous paths remain consistent with [src/adapters/http/auth_middleware.rs](../../src/adapters/http/auth_middleware.rs).

A.5 Query translation safety¶

Scope: [src/influxql/](../../src/influxql/), [src/adapters/chdb/](../../src/adapters/chdb/).

Must-pass

• Identifiers and literals are quoted/escaped so user input cannot break out of intended SQL structure.
• Timeouts / limits on query size or scan scope where exposed to untrusted clients.

A.6 Security¶

Must-pass

• Secrets not logged; TLS assumptions documented if termination is external.
• Password hashing uses argon2 as in [src/adapters/auth.rs](../../src/adapters/auth.rs).

A.7 Observability¶

Must-pass

• High-cardinality labels are not added to Prometheus metrics without review ([src/adapters/http/metrics.rs](../../src/adapters/http/metrics.rs)).

A.8 Cargo features¶

Scope: columnar-ingest and other cfg(feature = "...") in ingest/write paths.

Must-pass

• Default build (--no-default-features with empty default) still matches documented product behavior.
• Feature-gated code paths are tested (cargo test --all-features or dedicated job).

Layer B — Rust boundaries and maintainability¶

Must-pass

• ports traits remain the primary abstraction between application and adapters; domain types do not depend on HTTP or RocksDB directly.
• Config keys are documented in [docs/user-guide/configuration.md](../user-guide/configuration.md) and [config.toml.example](../../config.toml.example) when added or renamed.

Should-fix

• Very large binaries ([src/main.rs](../../src/main.rs)): new CLI or lifecycle logic should prefer modules under src/ over growing a single file (existing size may be scheduled refactors, not blocking every PR).

Nice-to-have

• Doc comments on new pub items; examples in developer guide where patterns repeat.

Layer C — Tests and operations¶

Suite ownership (avoid duplication)¶

Rule: Before deleting a test harness, show that another suite covers the same behavior and is run in CI or documented as manual-only.

CI coverage¶

• PR: Rust fmt, clippy, tests ([ci.yml](../../.github/workflows/ci.yml)).
• Container: build/push workflow ([container.yml](../../.github/workflows/container.yml))—consider mandatory for Dockerfile changes via policy, not necessarily every doc PR.

Supply chain¶

• Rust: commit Cargo.lock for the binary crate.
• Go: commit go.sum under hyperbytedb-operator/ and tests/e2e/.
• Pin major GitHub Actions (@v4, etc.) intentionally; upgrades are separate PRs.

Layer D — De-bloating criteria¶

Allowed removal classes (each needs a proof line in the PR):

1. Unused dependencies — cargo machete (or equivalent) clean + cargo check --all-targets and cargo test with relevant features.
2. Unreachable code — not part of stable pub API; grep/cargo doc consumers; integration tests still pass.
3. Duplicate documentation — consolidate with one canonical page; link from README.
4. Obsolete scripts — no references in CI, README, or deploy/; optional git log -- path note.
5. Local-only artifacts — never delete user data/, wal/, meta/ trees; respect .gitignore.

Breaking removals (HTTP shape, config keys, features): require explicit label and changelog note.

Appendix — Rust coding standards (Cursor / AI)¶

Single source of truth for rule content: the rust-skills rule set (MIT), summarized in .cursorrules for always-on IDE hints.

Optional local mirror: .cursor/skills/rust-skills/ (full per-rule files). Treat it as a cache for offline or deep linking; do not fork or diverge rule text without updating .cursorrules or documenting the drift in this appendix.

Contributors: see Engineering review artifacts and editor policy in the developer guide.