Review Standards¶

Every encoding must pass rigorous review before merging. This document provides the quality checklist, halachic and technical review criteria, common rejection reasons, and the revision process.

Quality Checklist¶

Pre-Submission Checklist¶

Before creating a PR, verify all items:

Source and Attribution¶

[ ] Every normative rule has @makor directive
[ ] At least 2 sources cited per rule (SA + Gemara or SA + Rambam)
[ ] D'oraita rules cite Torah source
[ ] Citation format is correct (sa("yd:87:1"))
[ ] madrega/2 specified for all normative rules
[ ] scope/2 specified for all rules

Rule Quality¶

[ ] Rule IDs are semantic with r_ prefix
[ ] No siman:seif based naming (e.g., r_87_1)
[ ] Override/3 uses descriptive values (never 'invalid')
[ ] World scoping is correct
[ ] Inheritance verified in child worlds

Testing¶

[ ] At least 5 positive test cases
[ ] At least 3 negative test cases
[ ] At least 2 edge cases
[ ] Multi-world tests (if applicable)
[ ] Machloket tests for both positions (if applicable)
[ ] All tests pass locally

Documentation¶

[ ] File header comment present
[ ] Section headers for rule groups
[ ] Inline comments for complex logic
[ ] Derivation chain documented
[ ] Machloket documented in comments

Technical¶

[ ] Type checker passes with no errors
[ ] Type checker passes with no warnings
[ ] No OWA predicate negation
[ ] Correct file location
[ ] Follows existing patterns in codebase

Halachic Review Criteria¶

Accuracy Verification¶

Reviewers check:

Criterion	Verification Method
Source accuracy	Compare encoding to original SA text
Madrega correctness	Verify d_oraita vs d_rabanan classification
Machloket representation	Confirm both positions are accurate
Override correctness	Verify override matches source disagreement
Scope appropriateness	Confirm world selection is correct

Source Verification Questions¶

Does the encoding match the SA text?
Read original SA seif
Compare encoded rule to text
Check for missing conditions
Is the madrega correct?
SA explicitly states d_oraita/d_rabanan?
Or derived from context?
Verify with Acharonim if unclear
Is the machloket real?
Does Rema actually disagree?
Or just add a case?
Check Taz/Shach for clarification
Is the override correct?
Does Rema override or just add?
Is the override in the right direction?
Do children inherit correctly?

Halachic Red Flags¶

These require immediate attention:

Red Flag	Action
D'oraita without Torah source	Request Torah citation
Machloket without both positions	Request missing position
Override with 'invalid' value	Reject, require descriptive value
Missing Rema check	Request Rema verification
Unsupported leniency	Request additional source

Technical Review Criteria¶

Code Quality¶

Criterion	Standard
Naming	Semantic, r_ prefix, no abbreviations
Structure	Grouped by topic, clear sections
Comments	Header, sections, inline for complexity
Patterns	Follows existing corpus patterns
Type safety	All predicates match registry

Architecture Compliance¶

File location
Corpus files in mistaber/ontology/corpus/yd_XX/
World overrides in mistaber/ontology/worlds/[world].lp
Tests in tests/corpus/yd_XX/
World structure
Uses correct world for scope
Inheritance is intentional
Override propagation is correct
Predicate usage
Uses existing predicates where available
New predicates are documented
Arity and sorts are correct

Technical Red Flags¶

These require immediate attention:

Red Flag	Action
Type checker errors	Reject until resolved
Type checker warnings	Request explanation or fix
OWA predicate negation	Reject, require positive condition
Missing tests	Reject until thresholds met
Broken inheritance	Request multi-world tests

Review Process¶

Review Workflow¶

graph TD
    A[PR Submitted] --> B[Automated Checks]

    B -->|Pass| C[Technical Review]
    B -->|Fail| D[Return for Fixes]

    C -->|Pass| E[Halachic Review]
    C -->|Fail| D

    E -->|Pass| F[Final Approval]
    E -->|Fail| D
    E -->|Questions| G[Discussion]

    G --> E

    F --> H[Merge]
    D --> I[Author Revises]
    I --> A

Reviewer Assignment¶

Content Type	Required Reviewers
D'oraita content	1 technical + 1 halachic
D'rabanan content	1 technical
New predicates	2 technical
Machloket	1 technical + 1 halachic
Helper predicates only	1 technical

Review Timeline¶

Phase	Expected Duration
Automated checks	< 5 minutes
Initial technical review	1-2 business days
Halachic review	2-3 business days
Revision response	1-2 business days
Final approval	1 business day

Common Rejection Reasons¶

Category 1: Source Issues¶

Reason	Example	Fix
Missing makor	No source citation	Add `makor(r_id, sa("..."))`
Wrong madrega	D'rabanan marked d_oraita	Verify from source
Unverified source	Citation doesn't exist	Verify and correct
Insufficient sources	Only 1 source for d_oraita	Add Gemara/Torah source

Category 2: Rule Issues¶

Reason	Example	Fix
Invalid override	`override(w, p, invalid)`	Use descriptive value
Bad rule name	`r_87_1`	Use `r_bb_beheima_achiila`
Wrong world	Rema ruling in mechaber	Move to correct world
Missing override	Child differs but no override	Add override predicate

Category 3: Test Issues¶

Reason	Example	Fix
Insufficient positive	Only 3 positive tests	Add 2 more
Missing negative	No negative tests	Add 3 negative tests
No edge cases	No boundary tests	Add 2 edge cases
Missing multi-world	Override not tested	Add child world tests

Category 4: Documentation Issues¶

Reason	Example	Fix
No file header	File starts with rules	Add header comment
Missing section comments	Rules without grouping	Add section headers
Undocumented complexity	Complex rule, no explanation	Add inline comments
Missing derivation	D_oraita without chain	Document derivation chain

Revision Process¶

Responding to Review Comments¶

Read all comments before starting revisions
Ask for clarification if any comment is unclear
Address every comment (fix or explain why not)
Mark resolved after addressing each comment
Request re-review when all addressed

Revision Best Practices¶

## Revision Response

### Comment 1: Missing Rema check
**Status**: Fixed
**Action**: Verified Rema has no gloss on this seif. Added comment
documenting that Rema's silence indicates agreement.

### Comment 2: Test coverage
**Status**: Fixed
**Action**: Added 3 additional edge case tests:
- test_empty_mixture
- test_single_food_no_mixture
- test_partial_categorization

### Comment 3: Consider using existing predicate
**Status**: Discussed
**Action**: The existing predicate does not cover this case because [reason].
Created new helper predicate as suggested alternative.

When to Request Re-Review¶

Request re-review when:

All comments have been addressed
New tests have been added and pass
Technical issues have been resolved
Additional source verification is complete

Do NOT request re-review when:

Comments are still unaddressed
New tests are failing
You have questions pending

Review Templates¶

Technical Review Template¶

## Technical Review: [PR Title]

### Automated Checks
- [ ] CI passes
- [ ] Type checker passes
- [ ] All tests pass

### Code Quality
- [ ] Rule IDs are semantic
- [ ] Correct file location
- [ ] Follows existing patterns
- [ ] Comments present

### World Structure
- [ ] Correct world scoping
- [ ] Override semantics correct
- [ ] Inheritance tested

### Tests
- [ ] 5+ positive tests
- [ ] 3+ negative tests
- [ ] 2+ edge cases
- [ ] Multi-world tests (if applicable)

### Issues Found
1. [Issue description]
2. [Issue description]

### Recommendation
- [ ] Approve
- [ ] Request changes
- [ ] Request halachic review

Halachic Review Template¶

## Halachic Review: [PR Title]

### Source Verification
- [ ] SA text verified
- [ ] Rema gloss checked
- [ ] Madrega confirmed
- [ ] Makor chain complete

### Accuracy Check
- [ ] Encoding matches source
- [ ] Machloket correctly represented
- [ ] Override direction correct
- [ ] No missing conditions

### Questions
1. [Question for encoder]

### Concerns
1. [Halachic concern]

### Recommendation
- [ ] Approve
- [ ] Request changes
- [ ] Request expert consultation

Approval Criteria¶

Minimum Requirements for Approval¶

Criterion	Threshold
CI status	All checks pass
Test count	Meets minimums
Source citations	All rules have makor
Technical review	At least 1 approval
Halachic review	Required for normative content

Merge Criteria¶

All of the following must be true:

All CI checks pass
Required approvals obtained
No unresolved comments
No merge conflicts
Branch is up to date with main

Post-Merge Verification¶

After merging:

Verify CI passes on main - Ensure integration is clean
Spot-check queries - Run sample queries to verify behavior
Monitor for issues - Watch for bug reports in following days
Document in changelog - Note significant additions

Testing Requirements - Test coverage details
Encoding Methodology - What reviewers expect
Halachic Accuracy - Accuracy standards
Contributing Encodings - Full workflow