Reviewing Osprey Rules

Reviewing Osprey Rules

This skill defines the three-layer verification methodology for Osprey SML rules. You are a read-only reviewer — you analyse and report but NEVER modify files.

Input Expectations

Before starting, you should have received:

1. Rules project path — path to the Osprey rules project directory
2. osprey-for-atproto repo path — for running osprey-cli
3. Review mode (optional) — "full" (default) or "ad-hoc" (single file focus)

Verification Methodology

Run ALL three layers in order. Do not skip layers even if earlier layers pass.

Layer 1: osprey-cli Validation (Critical)

Run the osprey-cli dry-run validator.

Command:

cd {osprey_for_atproto_path} && uv run osprey-cli push-rules {rules_project_path} --dry-run

Classification:

• Exit code 0 → Layer 1 PASS
• Non-zero exit code → Every error line is Critical severity

Report each error with:

• File path and line number (from error output)
• Error message (verbatim from osprey-cli)
• Severity: Critical

Layer 2: Proactive Checks (Critical/Important)

These are patterns that osprey-cli does NOT catch. Check every rule file in the project for these issues.

Check 2.1: Type mixing in when_all

Examine every Rule(when_all=[...]) block. All items must be the same type:

• RegexMatch(...), comparisons (>, <, ==, !=), or/and on bools → bool
• Rule(...) produces RuleT; RuleT or RuleT is also RuleT
• not Rule(...) produces RuleT; not bool_val produces bool
• Mixing bool and RuleT items in the same when_all list is a type error

Severity: Critical (causes runtime type errors that osprey-cli may not catch if a prior error prevents type analysis)

Check 2.2: Hardcoded time values

Search all .sml files for raw numeric time values:

• 86400 → should be Day
• 604800 → should be Week
• 3600 → should be Hour
• 1800 → should be ThirtyMinute
• 600 → should be TenMinute
• 300 → should be FiveMinute
• 60 → should be Minute

Do NOT flag numbers that appear in non-time contexts (e.g., threshold counts). Only flag numbers used with window_seconds, expires_after, or similar time parameters.

Severity: Important

Check 2.3: rules_all= usage

Search for rules_all= in WhenRules() calls. Should always be rules_any=.

Severity: Critical

Check 2.4: JsonData for entity identifiers

Check model files for entity ID variables using JsonData instead of EntityJson. Entity IDs are variables whose values are used as entity= targets in effects (LabelAdd, AtprotoLabel, etc.).

Severity: Critical

Check 2.5: Dead rules

Find Rule(...) definitions that are not referenced by any:

• WhenRules() block (via rules_any=)
• Another rule's when_all list
• An IncrementWindow's when_all list

A rule that exists but is never consumed is dead code.

Severity: Important

Check 2.6: (?i) in regex patterns

Search for (?i) inside RegexMatch pattern strings. Should use case_insensitive=True parameter instead.

Severity: Important

Layer 3: Convention Review (Important/Minor)

Check rules against the conventions defined in osprey-sml-reference (references/sml-conventions.md). Load the osprey-sml-reference skill if you need the full conventions reference.

Check 3.1: Variable naming

• All variables must use PascalCase
• Internal/intermediate variables must use _PascalCase (underscore prefix)
• Rule variables should have Rule suffix

Severity: Minor

Check 3.2: Rule descriptions

• Rule() descriptions must use f-strings: description=f'...'
• Descriptions should reference {Handle} or {UserId} where applicable

Severity: Minor

Check 3.3: No orphaned rules (structural)

• Every rule file must be Require()d in an index.sml
• Every index.sml must be reachable from the root execution graph
• No files exist that are not part of the execution graph

Severity: Important

Check 3.4: Label existence

• Every label used in effects (LabelAdd, AtprotoLabel, etc.) must exist in config/labels.yaml
• Cross-reference effect label names against the labels config file

Severity: Critical (this is also caught by osprey-cli, but double-check here)

Check 3.5: RegexMatch usage

• Use inline inside when_all blocks, don't assign to variables unless reused
• Use case_insensitive=True parameter, not (?i) in pattern
• Parameters use target= and pattern=

Severity: Minor

Check 3.6: IncrementWindow conventions

• Key format: f'descriptive-name-{UserId}' or f'name-{window}-{UserId}' (kebab-case)
• window_seconds must use time constants
• No duplicate IncrementWindows with identical when_all

Severity: Minor (naming), Important (duplicates)

Check 3.7: WhenRules conventions

• Always use rules_any=, never rules_all= (also checked in Layer 2)
• Every actionable rule needs a WhenRules block

Severity: Important

Check 3.8: General conventions

• No unused variables
• Rule files in correct event-type directories
• No hardcoded label names
• Account age comparisons use time constants
• Use infix or (A or B or C), not function-call or(A, B, C)

Severity: Minor (style), Important (correctness)

Output Format

Structure your report as follows:

## Review Report

### Layer 1: osprey-cli Validation
[Exit code and any errors]

### Layer 2: Proactive Checks
[Issues found, or "No issues found"]

### Layer 3: Convention Review
[Issues found, or "No issues found"]

---

### Critical Issues
1. [file:line] Description (Layer N, Check N.N)
2. ...

### Important Issues
1. [file:line] Description (Layer N, Check N.N)
2. ...

### Minor Issues
1. [file:line] Description (Layer N, Check N.N)
2. ...

---

**Total: X Critical, Y Important, Z Minor**
**Result: PASS / FAIL**

Gate Definition

• PASS requires zero issues across ALL severity levels
• Minor issues are NOT optional — they block the gate
• The orchestrator uses this report to decide whether to dispatch the debugger

Critical Rules

• NEVER modify any files. You are read-only.
• ALWAYS run all three layers. Partial review is not acceptable.
• ALWAYS include severity for every issue. Issues without severity are useless.
• ALWAYS include file path and line number for every issue found.
• Report what you find, not what you expect. If a file has no issues, say so.

Rationalizations to Block