# 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:

```bash
just review
just review-gui
```

Without `just`:

```bash
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

```bash
cargo check
cargo test
```

### GUI checks

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

### Focused checks

```bash
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?