Harden LOUDS trie topology validation

[zaidoon1]

Validate LOUDS trie topology-derived leaf counts while loading serialized trie data. Reject malformed blocks whose dense/sparse topology does not match the declared key count, child table count, or chain bitmap count before handle and seqno arrays are trusted.

This keeps validation on the cold InitFromData path and preserves the on-disk format, while making malformed custom index blocks fail closed with Corruption instead of allowing release builds to index handle/seqno arrays with unchecked topology-derived leaf ordinals.

[github-actions]

✅ clang-tidy: No findings on changed lines

Completed in 282.5s.

[github-actions]

Codex Code Review - OBSOLETE

Superseded by a newer AI review. Expand to see the original review.

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit f35b09d

---

❌ Codex review failed before producing findings.

WARNING: proceeding, even though we could not create PATH aliases: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

---

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

• Codex may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /codex-review [context] — Request a code review
• /codex-query <question> — Ask about the PR or codebase

[github-actions]

Claude Code Review - OBSOLETE

Superseded by a newer AI review. Expand to see the original review.

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit f35b09d

---

Summary

Well-structured defensive hardening of LOUDS trie deserialization. The validation formulas are mathematically correct, the edge cases (empty trie, all-dense, all-sparse, cutoff_level==0) are handled properly, and the changes are purely additive on a cold path with no performance concern. The test coverage is adequate for the key invariants but has some gaps.

High-severity findings (0):

No high-severity findings.

Full review (click to expand)

Findings

🔴 HIGH

No high-severity findings.

🟡 MEDIUM

M1. Test coverage gap: dense topology walk and several new checks are untested — trie_index_test.cc

• Issue: The PR adds ~16 distinct validation checks but only 5 new test cases. Several important new checks lack direct test coverage:
  • cutoff_level_ > max_depth_ + 1 — no test
  • computed_dense_leaf_count vs dense_leaf_count_ — tested indirectly via InitFromDataRejectsDenseLeafCountMismatch but only by patching dense_leaf_count_, not by corrupting the bitvector topology
  • dense_child_count_ vs expected_dense_child_count (the level-by-level walk) — no direct test
  • dense_nodes_seen != dense_node_count_ — no test
  • s_louds_.GetBit(0) root check — no test
  • s_is_prefix_key_.NumBits() != s_louds_.NumOnes() — no test
  • dense_child_count_ > s_louds_.NumOnes() — no test
  • s_child_end_pos_[k] <= s_child_start_pos_[k] (the < to <= change) — no test
  • s_louds_.GetBit(s_child_start_pos_[k]) — no test
• Root cause: Adding corruption tests for internal bitvector topology is complex (requires knowing exact byte offsets of bitvector contents), so only header-level field patching was tested.
• Suggested fix: Consider adding at least one test that corrupts dense_child_count_ in the header to exercise the level-by-level walk rejection, and one for cutoff_level_ > max_depth_ + 1. These are straightforward header patches at known offsets (offset 40 for dense_child_count, offsets 16/20 for cutoff/max_depth).

M2. Potential false rejection for edge case: cutoff_level_ > 0 with dense_node_count_ == 0 — louds_trie.cc

• Issue: In the dense topology walk (else branch when cutoff_level_ > 0), level_node_count is initialized to dense_node_count_ == 0 ? 0 : 1. If dense_node_count_ == 0 and cutoff_level_ > 0, the loop body never executes, dense_nodes_seen remains 0, and the check dense_nodes_seen != dense_node_count_ passes (0 == 0). Then expected_dense_child_count remains 0. This seems correct since no dense nodes means no dense children. However, this combination (cutoff_level_ > 0 with dense_node_count_ == 0) is impossible for a valid trie from the builder, since cutoff_level_ > 0 means at least the root node is dense. The check passes harmlessly but doesn't explicitly reject this invalid state.
• Root cause: The validation trusts the relationship between cutoff_level_ and dense_node_count_ is valid if other checks pass.
• Suggested fix: Consider adding an explicit check: if (cutoff_level_ > 0 && dense_node_count_ == 0) return Corruption(...). Low priority since downstream checks would catch any real problem.

🟢 LOW / NIT

L1. Ordering: cutoff_level_ validation before max_depth_ validation — louds_trie.cc:1062-1066

• Issue: The new cutoff_level_ > max_depth_ + 1 check is placed before the max_depth_ > kMaxReasonableDepth validation. While this is functionally correct (the uint64_t cast prevents overflow), placing it after the max_depth_ validation would be more natural — validate individual fields first, then validate relationships.
• Suggested fix: Move the cutoff_level_ check to after the max_depth_ validation. This is purely a style/readability suggestion.

L2. FindChainCountOffsetForTest assumes two child tables — trie_index_test.cc:1162-1184

• Issue: FindChainCountOffsetForTest computes child_table_padded + child_table_padded to skip both start and end position tables. This is correct but uses the same padding calculation for both tables, which only works because both have the same size (num_internal * sizeof(uint32_t)). This is always true, so it's not a bug, but the code would be clearer with separate variables.
• Suggested fix: No action needed. The code is correct.

L3. Test helper PatchFixed64ForTest uses ASSERT_LE — trie_index_test.cc:1106

• Issue: Using ASSERT_LE in a static helper function (not a test method) is fine for the current usage, but if the helper is called from a context where ASSERT failures should be non-fatal, EXPECT_LE would be more appropriate.
• Suggested fix: No action needed for current usage.

Cross-Component Analysis

Context	Does code execute?	Assumptions hold?	Action needed?
WritePreparedTxnDB	NO (trie index is SST-level, not txn-level)	N/A	None
ReadOnly DB	YES (SST index loading)	YES	None
CompactionService	YES (SST index loading)	YES	None
User-defined timestamps	YES (SST index loading)	YES	None
MemPurge	NO (trie index is on-disk)	N/A	None
BlobDB	YES (SST index loading)	YES	None
Concurrent access	NO (InitFromData is called once per SST open)	N/A	None

The validation is purely on the cold InitFromData path. All consumers of the validated fields (LoudsTrieIterator) are protected by the new checks. No thread-safety concerns since InitFromData is single-threaded.

Performance assessment: The dense topology walk is O(dense_node_count_) with 4 Rank1 calls per node. Rank1 is O(1). For typical SST files (16K-64K data blocks), the dense section has at most a few hundred nodes. This is negligible on a cold path that already does O(num_internal) work for child position table validation.

Positive Observations

• Correct overflow handling: The dense_node_count_ > max(uint64_t)/256 check prevents multiplication overflow before the comparison.
• Correct empty trie handling: All edge cases (num_keys_==0, cutoff_level_==0, dense_node_count_==0) are handled correctly.
• Guard removal is correct: Removing the if (dense_node_count_ > 0) and if (s_labels_size_ > 0) guards is safe because the checks become 0 != 0*256 → 0 != 0 → pass for empty sections.
• The < to <= change is correct: Every valid trie node must have at least one label, so child_end == child_start (zero labels) is invalid.
• Good use of static_cast<uint64_t> for the cutoff_level_ vs max_depth_ comparison to prevent uint32_t overflow when max_depth_ == UINT32_MAX.
• The num_keys_ == 0 ? 0 : 1 logic for cutoff_level_==0 correctly matches the builder's behavior where dense_child_count_ = 0 for empty tries and dense_child_count_ = 1 for non-empty all-sparse tries.
• Tests use proper helper functions with clear naming and reusable offset-finding logic.
• Defensive-in-depth approach: Multiple independent cross-checks (topology-derived leaf counts, child counts, chain bitmap counts) make it extremely difficult for a crafted input to pass all checks with inconsistent values.

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase

[github-actions]

Codex Code Review - OBSOLETE

Superseded by a newer AI review. Expand to see the original review.

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit 60ce3c0

---

❌ Codex review failed before producing findings.

WARNING: proceeding, even though we could not create PATH aliases: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

---

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

• Codex may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /codex-review [context] — Request a code review
• /codex-query <question> — Ask about the PR or codebase

[github-actions]

Claude Code Review - OBSOLETE

Superseded by a newer AI review. Expand to see the original review.

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit 60ce3c0

---

Summary

This is a well-designed defensive hardening PR that validates LOUDS trie topology during deserialization. The validation checks are mathematically sound, correctly handle edge cases (empty trie, all-sparse, all-dense), and remain on the cold InitFromData path. The test coverage is comprehensive with targeted mutation tests for each new check.

High-severity findings (0):
No high-severity findings.

Full review (click to expand)

Findings

🟡 MEDIUM

M1. Performance cost of dense level BFS walk on wide tries — louds_trie.cc:1156-1176

• Issue: The BFS walk to verify dense_child_count_ has O(dense_node_count) iterations in the inner loop, each calling Rank1 twice on d_labels_ and twice on d_has_child_. For a worst-case trie with ~16M dense nodes (the kMaxReasonableKeys limit), this performs ~64M rank operations during deserialization.
• Root cause: The walk re-derives the topology level by level to compute expected_dense_child_count, which requires touching every dense node.
• Suggested fix: This is on the cold InitFromData path (called once per SST open), so it may be acceptable. However, if profiling shows it's measurable for very large tries, consider caching level boundaries during serialization. Low priority unless benchmarks show impact.

M2. Missing validation: dense_node_count_ upper bound before overflow check — louds_trie.cc:1124

• Issue: The overflow guard dense_node_count_ > std::numeric_limits<uint64_t>::max() / 256 catches multiplication overflow, but there's no kMaxReasonable bound on dense_node_count_ itself (unlike num_keys_ and num_internal). A crafted dense_node_count_ close to UINT64_MAX / 256 would pass the overflow check but cause d_labels_.NumBits() comparison to succeed by coincidence with a crafted bitvector, then lead to the O(dense_node_count) BFS walk consuming unbounded CPU.
• Root cause: The kMaxReasonableKeys limit on num_keys_ indirectly bounds the trie size, but dense_node_count_ is read from untrusted data independently.
• Suggested fix: Add an explicit bound check: if (dense_node_count_ > kMaxReasonableDenseNodes) with a reasonable limit (e.g., 1ULL << 30), similar to num_keys_ and num_internal. This would also bound the BFS walk cost.

🟢 LOW / NIT

L1. cutoff_level_ cast safety is redundant but harmless — louds_trie.cc:1072-1075

• Issue: The cast static_cast<uint64_t>(cutoff_level_) > static_cast<uint64_t>(max_depth_) + 1 guards against max_depth_ + 1 wrapping to 0 when max_depth_ == UINT32_MAX. However, max_depth_ was already validated to be <= 65536 three lines above, so the uint32_t overflow cannot happen.
• Suggested fix: The cast is harmless and provides defense-in-depth. No change needed.

L2. Test helpers use hardcoded byte offsets for header fields — trie_index_test.cc:1964-1981

• Issue: Tests use magic numbers like /*max_depth offset=*/20, /*cutoff_level offset=*/16, /*dense_child_count offset=*/40. If the header format changes, all these offsets must be manually updated.
• Suggested fix: Define named constants for header field offsets (e.g., kHeaderNumKeysOffset = 8, kHeaderCutoffLevelOffset = 16). Low priority since the format is stable.

L3. FindChildPositionTableOffsetsForTest potential underflow — trie_index_test.cc:1195-1203

• Issue: table_padded > data.size() - end_offset could underflow if end_offset > data.size(). Only runs in test code on builder-produced data, so unreachable in practice.
• Suggested fix: Add an explicit end_offset > data.size() check before the subtraction.

L4. Nit: dense_node_count_ validation could be grouped with other header field limit checks — louds_trie.cc:1076-1081

• Issue: num_keys_ and max_depth_ are validated against reasonable limits immediately after reading. dense_node_count_ is read at line 1076 but not limit-checked until the bitvector comparison at line 1124.
• Suggested fix: See M2 above.

Cross-Component Analysis

Context	Does code execute?	Assumptions hold?	Action needed?
ReadOnly DB	YES (SST open)	YES	Safe
Secondary Instance	YES (SST open)	YES	Safe
WritePreparedTxnDB	YES (SST open)	YES	Safe
CompactionService	YES (SST open)	YES	Safe
User-defined timestamps	YES	YES	Safe
BlobDB	YES (SST open)	YES	Safe

Assumption Stress Test

Claim: "Removing the dense_node_count_ > 0 guard is safe"

• When dense_node_count_ == 0, all dense bitvectors have 0 bits for valid tries. The unconditional checks pass (0 == 0). For malformed files, the old code skipped these checks entirely; the new code catches the inconsistency. Strictly safer.

Claim: "cutoff_level_ == 0 sets expected_dense_child_count = num_keys_ == 0 ? 0 : 1"

• Builder sets dense_child_count_ = 0 for empty (louds_trie.cc:137) and = 1 for non-empty all-sparse (louds_trie.cc:468). Matches.

Claim: "Child position end <= start rejection"

• Internal nodes have fan-out >= 1, so end > start is required. Correct tightening.

Positive Observations

• Thorough cross-checking: header counts validated against bitvector topology bidirectionally
• BFS walk is a clever dense_child_count_ verification without extra metadata
• Tests are well-structured with targeted field-patching approach
• FindNthOneBit/NextSetBit usage in child position verification is safe due to prior dense_child_count_ + num_internal == s_louds_.NumOnes() check
• Overflow guard dense_child_count_ > UINT64_MAX - num_internal prevents addition overflow
• All checks remain on the cold path, preserving hot-path performance

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase

[github-actions]

Codex Code Review - OBSOLETE

Superseded by a newer AI review. Expand to see the original review.

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit 93fcd99

---

❌ Codex review failed before producing findings.

WARNING: proceeding, even though we could not create PATH aliases: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

---

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

• Codex may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /codex-review [context] — Request a code review
• /codex-query <question> — Ask about the PR or codebase

[github-actions]

Claude Code Review - OBSOLETE

Superseded by a newer AI review. Expand to see the original review.

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit 93fcd99

---

Summary

Well-structured defensive hardening PR that adds comprehensive topology validation to LoudsTrie::InitFromData(). The validation logic is correct, well-sequenced (earlier checks establish bounds for later bitvector operations), and correctly placed on the cold SST-open path. Tests are thorough with good coverage of corruption scenarios.

High-severity findings (0):
No high-severity findings.

Full review (click to expand)

Findings

🟡 MEDIUM

M1. Missing test coverage for several new validation checks — trie_index_test.cc

• Issue: Several new validation paths lack dedicated corruption tests:
  • Dense level-walk overflow (level_start_node + level_node_count > dense_node_count_)
  • dense_nodes_seen != dense_node_count_ mismatch
  • s_louds_.GetBit(0) sparse root check
  • s_is_prefix_key_.NumBits() != s_louds_.NumOnes()
  • dense_child_count_ > s_louds_.NumOnes()
  • Total leaf count mismatch (dense_leaf_count_ + computed_sparse_leaf_count != num_keys_)
  • s_louds_.NumOnes() != dense_child_count_ + num_internal (overflow-guarded check)
  • The <= change for child position end vs start (zero-size node)
• Root cause: These checks are exercised indirectly by some tests (e.g., patching num_keys_ triggers the leaf count check), but they don't have dedicated tests that verify the specific error path fires.
• Suggested fix: Add targeted tests for each untested path. Priority: the s_louds_.GetBit(0) check and total leaf count check.

🟢 LOW / NIT

L1. Dense level-walk loop is O(dense_node_count_) with 4 Rank1 calls per node — louds_trie.cc

• Issue: For adversarially crafted input at kMaxReasonableDenseNodes (~16M nodes), this is ~64M Rank1 calls (~100ms). Acceptable for the cold InitFromData path, and the limit is far beyond realistic SSTs (typical: 1-100 dense nodes).
• Suggested fix: No change needed.

L2. Redundant overflow check for dense_node_count_ * 256 — louds_trie.cc

• Issue: The dense_node_count_ > UINT64_MAX / 256 check is redundant after kMaxReasonableDenseNodes enforcement. Harmless defense-in-depth.
• Suggested fix: No change needed.

L3. Test helper FindChildPositionTableOffsetsForTest bounds logic is correct but subtle — trie_index_test.cc

• Issue: The data.size() - end_offset subtraction is safe but requires careful reading of the preceding bounds check to verify.
• Suggested fix: No change needed; could add a brief comment.

Cross-Component Analysis

All execution contexts (WritePreparedTxnDB, ReadOnly DB, CompactionService, BlobDB, etc.) use the same TrieIndexReader::InitFromSlice() → LoudsTrie::InitFromData() path. No context-specific assumptions in the validation code.

Key correctness verifications:

1. Empty trie: Removing if (dense_node_count_ > 0) / if (s_labels_size_ > 0) guards is safe — zero values pass all checks.
2. cutoff_level_==0: expected_dense_child_count = num_keys_ == 0 ? 0 : 1 matches builder behavior.
3. Dense leaf count formula: Matches SuRF paper. Subtraction cannot underflow.
4. Sparse node count invariant: s_louds_.NumOnes() == dense_child_count_ + num_internal is correct.
5. Child position <= change: Correct — nodes must have ≥1 label.
6. Bitvector operation safety: All calls have valid arguments, proven by preceding checks.

Positive Observations

• Defense-in-depth overflow checks strengthen against future refactoring
• Well-sequenced validation ensures bitvector operation safety
• No hot-path impact — all changes on cold SST-open path
• No on-disk format changes — purely additive validation
• Good test helper design for navigating serialized format

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase

[github-actions]

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit f45649c

---

❌ Codex review failed before producing findings.

WARNING: proceeding, even though we could not create PATH aliases: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

---

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

• Codex may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /codex-review [context] — Request a code review
• /codex-query <question> — Ask about the PR or codebase

[github-actions]

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit f45649c

---

Summary

Solid defensive hardening PR that adds comprehensive topology validation to LoudsTrie::InitFromData(). The validation catches structural inconsistencies in serialized trie data before they can cause OOB array accesses during traversal. The changes are on the cold path (block load), preserve the on-disk format, and include thorough test coverage for each new check.

High-severity findings (0):

No high-severity findings.

Full review (click to expand)

Findings

🟡 MEDIUM

M1. Bitvector num_ones_ is not verified against actual popcount — pre-existing but relevant — bitvector.cc:91-198

• Issue: Bitvector::InitFromData reads num_ones_ from the serialized header and trusts it without recomputing from the actual word data. The new topology validation uses NumOnes() (e.g., d_labels_.NumOnes(), d_has_child_.NumOnes()) which returns this potentially spoofed value. If both num_ones and the rank LUT are consistently crafted, validation passes but topology data is semantically wrong.
• Root cause: Pre-existing design in Bitvector — not introduced by this PR. Block checksums are the primary defense against crafted data; this validation is defense-in-depth.
• Suggested fix: Out of scope for this PR. A future PR could add popcount verification in Bitvector::InitFromData.

M2. Dense level walk loop has O(dense_node_count) cost with 16M upper bound — louds_trie.cc (new code)

• Issue: The level walk iterates over all dense nodes calling Rank1() twice per node. With kMaxReasonableDenseNodes ≈ 16M, worst case is ~32M Rank1 calls (~160ms). However, a block with 16M dense nodes would itself be ~512MB, making this impractical through normal SST files.
• Suggested fix: No action needed. The existing bound is sufficient.

🟢 LOW / NIT

L1. Redundant overflow guard before dense_node_count_ * 256 — louds_trie.cc

• Issue: The dense_node_count_ > UINT64_MAX / 256 guard is redundant given the earlier kMaxReasonableDenseNodes check. Harmless but unnecessary.

L2. Consider testing zero-key trie validation — trie_index_test.cc

• Issue: No test verifies that a valid zero-key trie passes all new validation checks.

Cross-Component Analysis

All execution contexts (ReadOnly DB, WritePreparedTxnDB, user-defined timestamps, BlobDB, various compaction styles) are safe — InitFromData is a pure deserialization function with no context-dependent behavior.

Key assumptions verified:

• d_labels_.NumOnes() - d_has_child_.NumOnes() cannot underflow (guarded by prior NumBits == NumOnes check)
• (node_num + 1) * 256 cannot overflow uint64_t (bounded by kMaxReasonableDenseNodes)
• FindNthOneBit(child_node) is always in-bounds (guarded by dense_child_count_ + num_internal == s_louds_.NumOnes())
• s_louds_.GetBit(0) is guarded by s_labels_size_ > 0 check
• Change from < to <= in child position check correctly rejects empty LOUDS node spans

Positive Observations

1. Excellent defense-in-depth: fail-fast on cold path prevents hot-path OOB accesses
2. Comprehensive topology cross-checks go beyond header field validation
3. Well-structured tests with purpose-built helper functions
4. Zero hot-path performance impact
5. Fully backward compatible

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase