geolog-zeta-fork/REVIEWING.md

Reviewing Geolog

This guide is for humans reviewing the codebase, especially when you want to answer:

• where a behavior lives,
• which commands to run first,
• and which modules are most likely to matter for a given bug or change.

Quick Start

If you have just installed:

just review
just review-gui

Without just:

cargo check
cargo test
cargo check --features gui --bin geolog-gui
cargo test --features gui

Best Reading Order

For most reviews, this order gives the fastest mental model:

1. README.md
   • User-facing behavior and examples.
2. docs/ARCHITECTURE.md
   • High-level subsystem map.
3. src/lib.rs
   • Top-level module layout.
4. Syntax pipeline
   • src/ast.rs
   • src/lexer.rs
   • src/parser.rs
   • src/pretty.rs
5. Semantic core
   • src/core.rs
   • src/id.rs
   • src/universe.rs
6. Elaboration
   • src/elaborate/mod.rs
   • src/elaborate/theory.rs
   • src/elaborate/instance.rs
7. Runtime and persistence
   • src/repl.rs
   • src/store/mod.rs
   • src/store/theory.rs
   • src/store/instance.rs
8. Query and chase
   • src/query/mod.rs
   • src/query/chase.rs
   • src/query/compile.rs
   • src/query/backend.rs
9. Optional layers
   • src/solver/
   • src/gui/

How To Map A Change

Use this shortcut table:

• Parse error or syntax ambiguity
  • Start in src/lexer.rs and src/parser.rs.
• Pretty-print or round-trip bug
  • Start in src/pretty.rs, then tests/unit_pretty.rs and tests/unit_parsing.rs.
• Wrong theory or instance semantics
  • Start in src/elaborate/ and src/core.rs.
• Persistence or workspace bug
  • Start in src/repl.rs and src/store/.
• Chase/query behavior bug
  • Start in src/query/chase.rs, src/query/compile.rs, and related tests.
• GUI-only behavior bug
  • Start in src/gui/state.rs, then src/gui/panels/ or src/gui/visualizations/.

Important Invariants

• parser should stay syntax-focused.
  • Semantic decisions generally belong in elaboration, not parsing.
• core should remain GUI-free.
  • GUI code should consume existing semantics, not define them.
• GUI must remain feature-gated.
  • GUI code lives behind the gui feature.
• Persistence changes need extra care.
  • src/store/ and related runtime flows should preserve expected workspace behavior.
• Chase/query changes need explicit tests.
  • These regressions are often subtle and easy to miss in manual review.

Best Review Commands

Start narrow, then go broad.

Core checks

cargo check
cargo test

GUI checks

cargo check --features gui --bin geolog-gui
cargo test --features gui

Focused checks

cargo test unit_parsing
cargo test unit_elaborate
cargo test unit_chase
cargo test unit_workspace
cargo test gui_

Test File Guide

• tests/unit_parsing.rs
  • Parser and syntax behavior.
• tests/unit_pretty.rs
  • Pretty-printing and round-trip expectations.
• tests/unit_elaborate.rs
  • Theory/instance elaboration semantics.
• tests/unit_chase.rs
  • Chase behavior.
• tests/unit_workspace.rs
  • Persistence/workspace behavior.
• tests/examples_integration.rs
  • End-to-end examples.
• tests/proptest_*.rs
  • Property-based coverage.

What Usually Deserves Extra Scrutiny

• Changes that cross parser → elaboration boundaries.
• Changes in src/core.rs because they affect many layers.
• Anything in src/query/chase.rs, src/query/backend.rs, or src/store/.
• GUI changes that mutate runtime state rather than only presenting it.
• Feature-gated code that may compile less often than the default build.

Useful Missing Context To Watch For

If a patch is hard to review, it often needs one of these:

• a smaller regression test,
• a more focused example in examples/geolog/,
• a note in docs/ARCHITECTURE.md,
• or a short comment explaining a non-obvious invariant.

Reviewer Checklist

• Does the change live in the right layer?
• Are semantics changed intentionally, or just presentation?
• Is there a narrow test for the bug or behavior?
• If GUI code changed, is the same state transition safe from the REPL path too?
• If store/query/chase code changed, do existing tests still cover the affected behavior?
• Is any documentation now stale?