Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,28 @@
## Problem and change
## Problem and why it matters

Explain the problem, why the change is needed, and how it helps users. Link related issues.
What problem does this solve, who runs into it, and what shows it on `main` (a reproducer, a profile or a
benchmark run)? Why does the fix belong in lance-csr, and what does it cost? Link related issues.

## Change

What the change does, and any approaches you tried and rejected.

## Validation

List the checks run and their results. For bug fixes, include a regression test.
For code changes, include correctness checks and before-and-after results for both benchmark
suites, with hardware and settings. Documentation-only changes do not need benchmarks.
List the checks you ran and their results. For a bug fix, include a regression test.

For a code change, include results from both benchmark suites, built with the harness in
[CONTRIBUTING.md](https://github.com/hyperdimensionalcomputingai/lance-csr/blob/main/CONTRIBUTING.md#build-the-benchmark-harness).
For each suite and mode (`memory`, `lazy`), report every query:

| Query | `main` (ms) | branch (ms) | Change |
| --- | ---: | ---: | ---: |
| | | | |

Add the geometric mean of the ratios, the gap between the two `main` runs, the commits of lance-csr and
both benchmark repositories, and the machine, OS and memory. Explain any queries that got noticeably slower.
Documentation-only changes don't need benchmarks.

## User impact

Note any API or behaviour changes and the documentation or examples updated, or write “None”.

See [contribution guidelines](https://github.com/hyperdimensionalcomputingai/lance-csr/blob/main/CONTRIBUTING.md).
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@
- Follow the existing history style (`feat(graph):`, `docs:`, `refactor(query):`), using imperative, ≤72-character subjects.
- Reference issues or discussions when relevant and include brief context in the body.
- PRs should describe scope, list test commands run, mention benchmark deltas when applicable, and highlight impacts on bindings or examples.
- Before a PR claims a code change is faster, or doesn't slow anything down, follow `CONTRIBUTING.md`: explain the problem and why the change is needed, and measure the branch against `main` on both benchmark suites with its harness (release wheels loaded with `uv run --with`, both `memory` and `lazy` modes). Never edit benchmark queries, expected answers or runner settings, and report every query.
242 changes: 224 additions & 18 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,25 +1,231 @@
# Contributing to lance-csr

Start with the [project goals](README.md#goals) and [contribution guidelines](README.md#contributing).
This project focuses on single-machine graph queries; distributed execution is out of scope.
For a larger change, open an issue to explain the problem and proposed approach before starting work.
lance-csr aims to be a fast, correct, **single-machine** Cypher engine on Lance (see the
[project goals](README.md#goals)). Distributed execution is out of scope. Contributions within those goals
are welcome, from people and from their coding agents.

## Report a bug or suggest an improvement
What matters in a pull request:

- **A clear reason:** the problem it solves, or the queries it makes faster.
- **Correct results:** every test and benchmark answer still checks out.
- **Evidence:** a regression test for a bug fix; before-and-after numbers from both benchmark suites for a
code change.
- **No surprises:** no unexplained slowdowns elsewhere.

The sections below cover each of these, including a step-by-step benchmark harness you can hand to a coding
agent.

## Start with the why

Before writing code, be able to answer these questions. Your pull request description should answer them
too.

1. **What is the problem?** A query that returns the wrong rows, fails, or runs slowly. Show it.
2. **Who runs into it?** A workload, a query shape, a data size. "It could be faster" is not a problem yet.
3. **What is the evidence?** A reproducer, a profile, or a benchmark run that shows the problem on `main`.
4. **Why does the fix belong in lance-csr?** Rather than in DataFusion, Lance, or the user's query.
5. **What does it cost?** New code paths, memory, build time, API surface.
6. **What else did you try?** Approaches you rejected, and why.

For a larger change, open an [issue](https://github.com/hyperdimensionalcomputingai/lance-csr/issues) with
these answers before you start, so that we can agree on the approach before you spend time on it.

## What evidence each change needs

| Change | Evidence |
| --- | --- |
| Bug fix | A regression test that fails on `main` and passes on your branch. |
| Performance improvement | Before-and-after results from both benchmark suites, built with the [harness below](#build-the-benchmark-harness). |
| New feature or Cypher support | Tests, documentation, and benchmark results showing that existing queries didn't get slower. |
| Refactor | Tests pass, and benchmark results if it touches planning, execution or the CSR. |
| Documentation only | None. |

Every code change also needs the usual checks to pass; see [Set up, test and lint](#set-up-test-and-lint).

## The benchmark suites

We guide performance work with two suites over the GDC Social Network Benchmark SF1 dataset (3.18M nodes,
17.3M relationships). Each lives in its own repository, next to the runners for the other engines, and every
query checks its result against an expected answer:

- [graph-benchmark-ldbc](https://github.com/hyperdimensionalcomputingai/graph-benchmark-ldbc): 30 custom
Cypher queries. lance-csr's runner is in `lance_csr/`, a uv project of its own.
- [graph-benchmark-ldbc-complex](https://github.com/hyperdimensionalcomputingai/graph-benchmark-ldbc-complex):
the 14 Interactive complex read queries (IC1–IC14). lance-csr's runner is in `lance_csr/` and uses the
repository's root uv project.

Both run lance-csr in two modes, selected by `LANCE_CSR_MODE`: `memory` holds every table as Arrow in memory,
and `lazy` reads the Lance datasets from disk as queries need them. A change can speed up one mode and slow
down the other, so measure both.

The micro-benchmarks in [`python/benchmarks/`](python/benchmarks/) and
[`crates/lance-csr-benches/`](crates/lance-csr-benches/README.md) are useful for isolating one operator,
and you're welcome to include their numbers. They support a claim; they don't replace the two suites.

## Build the benchmark harness

The runners pin `lance-csr` to a release from PyPI. Running `./run_benchmarks.sh` as it is measures that
release, not your branch. The harness below builds two wheels, one from `main` and one from your branch, and
runs the same runner against each with `uv run --with`, which loads the wheel you name in place of the pinned
release and leaves the runner's files untouched.

### 1. Set up the suites once

Clone both benchmark repositories next to your lance-csr checkout, and follow their READMEs to download the
SF1 dataset and build the Lance datasets: `lance_graph/` in graph-benchmark-ldbc (its `lance_csr/` runner
reads `../lance_graph/graph_lance`), and `lance_csr/build_graph.py` in graph-benchmark-ldbc-complex. This is
the slow part, and you only do it once.

```text
code/
├── lance-csr/
├── graph-benchmark-ldbc/
└── graph-benchmark-ldbc-complex/
```

### 2. Build a wheel from `main` and one from your branch

From `lance-csr/python/`, with the development environment set up (`make install`, see
[`python/DEVELOPMENT.md`](python/DEVELOPMENT.md)):

```bash
export BENCH=~/lance-csr-bench
git switch main
rm -rf "$BENCH/wheels/main" && uv run --no-sync maturin build --release --out "$BENCH/wheels/main"
git switch my-branch
rm -rf "$BENCH/wheels/branch" && uv run --no-sync maturin build --release --out "$BENCH/wheels/branch"
```

Always benchmark release builds; debug builds run queries many times slower. Each release build takes
several minutes. Both wheels report the same version, so write down the commit each one was built from.
Rebuild the branch wheel after every change you want to measure.

### 3. Run each suite against each wheel

From a benchmark repository's root, this runs one suite, in one mode, against one wheel, with the settings
from that repository's `run_benchmarks.sh`, and saves the results under `$BENCH/runs`:

Search the [existing issues](https://github.com/hyperdimensionalcomputingai/lance-csr/issues) first.
For bugs, include a small reproducer, expected and actual results, your lance-csr and Python versions,
and your operating system. Use synthetic data when possible; remove credentials and private data.
For improvements, describe the use case and how the change would help users.
```bash
bench() { # usage: bench <main|branch> <memory|lazy>
local wheel=$(ls "$BENCH/wheels/$1"/lance_csr-*.whl 2>/dev/null)
[ -f "$wheel" ] || { echo "no single wheel in $BENCH/wheels/$1" >&2; return 1; }
local suite=$(basename "$PWD")
(cd lance_csr && LANCE_CSR_MODE=$2 uv run --frozen --with "$wheel" \
pytest benchmark_query.py -p no:faulthandler \
--benchmark-min-rounds=5 --benchmark-min-time=0.000005 --benchmark-max-time=1.0 \
--benchmark-timer=time.perf_counter --benchmark-calibration-precision=10 \
--benchmark-warmup=off --benchmark-warmup-iterations=5 --benchmark-disable-gc \
--benchmark-sort=fullname \
--benchmark-storage="file://$BENCH/runs/$suite" --benchmark-save="$1-$2")
}
```

Before timing anything, check that the runner loads the wheel and not the pinned release. The path printed
should be in uv's cache, not in the runner's `.venv`:

```bash
(cd lance_csr && uv run --frozen --with "$BENCH"/wheels/branch/lance_csr-*.whl \
python -c "import lance_csr; print(lance_csr.__file__)")
```

Then alternate the builds, so that a change in machine load affects both equally, and run each build twice:

```bash
for mode in memory lazy; do
bench main $mode; bench branch $mode; bench main $mode; bench branch $mode
done
```

Run this in both repositories. Keep the machine otherwise idle and plugged in, and use the same machine for
every run. Each run's tests check results against the expected answers: a failing test means the change is
wrong, however fast it is.

### 4. Compare

`pytest-benchmark compare` lists the saved runs side by side, one table per query. From a benchmark
repository's root, for the `memory` runs (use `'*/*-lazy'` for the others):

```bash
suite=$(basename "$PWD")
(cd lance_csr && uv run --frozen pytest-benchmark --storage "file://$BENCH/runs/$suite" \
compare '*/*-memory' --group-by=name --columns=mean,stddev --time-unit=ms)
```

Each row is labelled with the start of its run's name, such as `0001_main-me` or `0002_branch-`. The saved
JSON files (`$BENCH/runs/<suite>/<machine>/*.json`) hold every statistic, if you or your agent want to compute
the summary below with a script.

For each suite and mode, report the mean latency per query on `main` and on your branch, the change, and
the geometric mean of the per-query ratios. The difference between the two `main` runs tells you how much
the numbers move on your machine without any code change.

### How to read the results

Queries in these suites take from under a millisecond to about a second, and a query's time can move by
15–20% between runs on the same machine, in either direction. Treat changes within that range as noise:
don't claim them as improvements, and we won't hold them against you as regressions.

Changes beyond that range are worth looking at. A real improvement shows up in both runs of your branch, by
more than the gap between the two `main` runs. If several queries get slower by more than that range,
investigate before you open the pull request: profile them, find out why, and explain it in the description.
A regression with a good explanation, traded for a larger gain, can still be the right change; we'll work it
out with you in review.

## A brief for your coding agent

To have an agent produce the evidence, give it this guide and a brief like this one:

> Measure the performance effect of the branch `my-branch` in the lance-csr checkout at `<path>`, following
> "Build the benchmark harness" in its `CONTRIBUTING.md`. Build release wheels from `main` and from the
> branch. Before timing, confirm the runner imports each wheel rather than the pinned release. Run both
> suites, in both `memory` and `lazy` modes, alternating `main` and branch runs twice each.
> Don't edit the benchmark queries, expected answers or runner settings. Report every query, not only those
> that improved: for each suite and mode, a table of mean latency on `main` and on the branch with the
> change, the geometric mean of the ratios, and the gap between the two `main` runs. List any failing
> tests. Record the commits of lance-csr and both benchmark repositories, and the machine, OS and memory.

Read what the agent reports before you post it. You're the author of the pull request, and its numbers have to
hold up when someone else reproduces them.

## Why it's worth the effort

- **Pull requests with reproducible numbers get reviewed first.** A table we can reproduce answers most of
the questions we would otherwise ask in review.
- **Without numbers, we'll ask for them.** A performance pull request without results waits until it has
them.
- **Finding a problem is a contribution.** If the harness shows a query where lance-csr is slow or wrong,
an issue with the query and your numbers is valuable on its own, even without a fix. So is a new query
added to one of the benchmark repositories that covers a workload they miss.

## Set up, test and lint

[Contributing](docs/contributing.md) in the docs covers the setup (Rust, `protoc`, Python, uv), the test and
lint commands, and how to build the documentation site; [`python/DEVELOPMENT.md`](python/DEVELOPMENT.md) has
the details of the Python build. In short, never use `pip`, and run these before you push:

```bash
cargo test --workspace
cargo clippy --workspace --all-targets -- -D warnings
cargo fmt --all --check
cd python && make test && make lint
```

When you change user-facing behaviour or the Python API, update the affected guide pages and
`docs/api-reference.md` in the same pull request, then run
`uv run --project docs --locked zensical build --strict` and `uvx typos@1.26.0 .`.

## Report a bug or suggest an improvement

## Prepare a pull request
Search the [existing issues](https://github.com/hyperdimensionalcomputingai/lance-csr/issues) first. For a
bug, include a small reproducer, the expected and actual results, your lance-csr and Python versions, and your
operating system. Use synthetic data when you can, and remove credentials and private data. For an
improvement, describe the use case and how the change would help users.

Follow the [build instructions](README.md#build-from-source) and
[Python development guide](python/DEVELOPMENT.md) to set up and run the relevant checks.
Keep changes focused, add a regression test for bug fixes, and update affected documentation for
user-visible changes. Python API changes also need an update to `docs/api-reference.md`.
## Open a pull request

Explain the problem, why the change is needed, and how you validated it. Code changes need correctness
checks and before-and-after results from both benchmark suites listed in the
[contribution guidelines](README.md#contributing), using the same hardware and settings.
Documentation-only changes do not need benchmarks. Agent-assisted contributions follow the same
requirements: review the generated code and validate it before submitting.
- Keep it focused on one change, and follow the commit style in the history (`feat(graph):`, `fix(cypher):`,
`docs:`, `refactor(query):`), with an imperative subject of at most 72 characters.
- Fill in the [pull request template](.github/pull_request_template.md): the problem and why the change is
needed, the checks you ran, the benchmark tables, and the effect on users.
- Agent-assisted pull requests are held to the same standard. Review the code and the numbers yourself
before you submit.
19 changes: 5 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -311,20 +311,11 @@ Mean query latency in milliseconds (lower is better, fastest per query in bold):

## Contributing

Contributions within the [project's goals](#goals) are welcome! Please explain the problem your PR solves,
why the change is needed, and how it helps users. Agent-assisted contributions are welcome too: bring a clear
motivation, review the generated code, and validate it. We may close PRs that lack a concrete problem or rationale;
if you're unsure whether an idea fits, open an issue to discuss it first.

We guide improvements with empirical evidence. Before submitting a code change, benchmark it against `main`
on both suites, using the same hardware and settings for each comparison:

- [GDC SNB SF1, 30-query workload](https://github.com/hyperdimensionalcomputingai/graph-benchmark-ldbc).
- [GDC SNB Interactive complex queries (IC1–IC14)](https://github.com/hyperdimensionalcomputingai/graph-benchmark-ldbc-complex).

Include a before-and-after results table, correctness checks, and enough setup details to reproduce the runs in
your PR. We only accept changes that preserve correctness and avoid significant performance regressions in
either suite. Documentation-only changes do not need benchmark runs.
Contributions within the [project's goals](#goals) are welcome, from people and from their coding agents. Every
pull request should explain the problem it solves and why the change is needed, and back that up with evidence:
a regression test for a bug fix, and before-and-after results from both benchmark suites for a code change.
[`CONTRIBUTING.md`](CONTRIBUTING.md) explains what to include and how to build the benchmark harness that
measures your branch against `main`. If you're unsure whether an idea fits, open an issue to discuss it first.

## Credits

Expand Down
6 changes: 5 additions & 1 deletion docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ behaviour, update the guide and the [API reference](api-reference.md) in the sam
an imperative subject of at most 72 characters.
- Describe the scope, list the test commands you ran, and note any impact on the Python bindings, examples or
docs.
- For performance-sensitive changes, include numbers from [`python/benchmarks/`](benchmarks.md#engine-micro-benchmarks).
- Explain the problem the change solves and why it's needed. For a code change, include before-and-after
results from both [benchmark suites](benchmarks.md), measured against `main` with the harness described in
[`CONTRIBUTING.md`](https://github.com/hyperdimensionalcomputingai/lance-csr/blob/main/CONTRIBUTING.md#build-the-benchmark-harness).
Numbers from [`python/benchmarks/`](benchmarks.md#engine-micro-benchmarks) can support a claim but don't
replace the two suites.

See [Project structure](project_structure.md) for a map of the code.
Loading