Skip to content

Commit 00730aa

Browse files
committed
docs(review): tighten reproducibility guarantees
1 parent bc962fc commit 00730aa

1 file changed

Lines changed: 15 additions & 7 deletions

File tree

‎docs/review-context.md‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22

33
`codebase-context-review` turns a committed git diff into a bounded review-context packet.
44

5-
It is deliberately **not** an AI reviewer. It does not call an LLM, decide whether code is correct, or post comments. Its job is narrower and testable: compile the changed surface, stable diff signals, related codebase context, and current conventions into one reproducible input for a reviewer or evaluation harness.
5+
It is deliberately **not** an AI reviewer. It does not call an LLM, decide whether code is correct, or post comments. Its job is narrower and testable: compile the changed surface, stable diff signals, related codebase context, and current conventions into a versioned input for a reviewer or evaluation harness.
66

77
## Why this exists
88

9-
A PR reviewer needs more than the patch, but dumping an entire repository into a model is expensive and hard to reproduce. The review-context path creates a deterministic boundary between:
9+
A PR reviewer needs more than the patch, but dumping an entire repository into a model is expensive and hard to reproduce. The review-context path creates a clean boundary between:
1010

1111
1. **context compilation** — git + local repository analysis
1212
2. **review reasoning** — any model or human reviewer consuming the packet
@@ -20,7 +20,7 @@ Build and run from the repository:
2020

2121
```bash
2222
pnpm build
23-
node dist/review-cli.js --base origin/main --head HEAD
23+
node dist/review-bin.js --base origin/main --head HEAD
2424
```
2525

2626
After package publication, the package also exposes:
@@ -42,15 +42,18 @@ npx codebase-context-review \
4242

4343
The command uses merge-base diff semantics (`base...head`), which matches the normal pull-request question: what changed on this branch since it diverged from the base branch?
4444

45+
`review-context-v1` requires `--head` to resolve to the checked-out `HEAD` and requires a clean working tree. The diff itself is then generated from the already-resolved commit SHAs rather than mutable symbolic refs.
46+
4547
## Packet contract
4648

4749
The current schema is `review-context-v1`.
4850

4951
The packet contains:
5052

5153
- exact resolved base/head commit SHAs
52-
- SHA-256 fingerprint of the raw git diff
54+
- SHA-256 fingerprint of Git's exact raw diff output
5355
- changed file status, additions/deletions, rename source, and binary flag
56+
- NUL-safe filename parsing for unusual valid git paths
5457
- identifiers extracted deterministically from changed lines
5558
- bounded search queries derived from those diff signals
5659
- bounded related-context results from the existing `search_codebase` engine
@@ -60,6 +63,8 @@ The packet contains:
6063

6164
Absolute local repository paths are not part of the packet contract.
6265

66+
The git envelope, identifier extraction, bounds, and query derivation are deterministic. Related-context ranking is only reproducible under the same `codebase-context` version, index contents, embedding/reranking configuration, and runtime dependencies; benchmark manifests should freeze those inputs rather than pretending the entire retrieval stack is environment-independent.
67+
6368
## Bounds
6469

6570
Defaults:
@@ -75,13 +80,15 @@ These are explicit because an unbounded context compiler is not useful evidence.
7580

7681
## Index behavior
7782

78-
If the repository has no existing codebase-context index, the command indexes it before searching. Pass `--no-index` to fail instead, which is useful in controlled benchmark runs where setup/index cost must be measured separately.
83+
By default, the command creates an index when one is missing and runs an incremental refresh when one already exists. This keeps normal use aligned with the checked-out clean `HEAD`.
84+
85+
Pass `--no-index` to prohibit both creation and refresh. It fails if no index exists. This is useful in controlled benchmark runs where setup/index work is captured separately; the caller is then responsible for proving that the supplied index matches the frozen source state.
7986

8087
The command is local-first. It invokes git and the existing local index/search pipeline; it does not introduce an LLM or external review API.
8188

8289
## What this proves
8390

84-
Shipping this command proves only that the project can compile a deterministic review-oriented context packet from a real git range.
91+
Shipping this command proves only that the project can compile a bounded, versioned review-oriented context packet from an exact committed git range.
8592

8693
It **does not** prove that the packet improves review quality, catches more bugs, reduces false positives, or beats another context strategy. Those are benchmark claims and remain blocked until measured.
8794

@@ -102,8 +109,9 @@ The existing ContextBench protocol already follows the same evidence discipline
102109
## Current limitations
103110

104111
- Only committed git refs are supported in v1; working-tree/staged review is intentionally deferred.
112+
- `--head` must be the checked-out commit and the worktree must be clean.
105113
- Query generation is lexical and deterministic. It extracts identifiers from changed lines and falls back to path signals; it is not AST-aware yet.
106-
- The command searches the current repository index. A stale index can therefore produce stale related context; search-quality/preflight output should be preserved by consumers.
114+
- `--no-index` deliberately skips freshness work, so benchmark callers must attest the index/source match themselves.
107115
- Large diffs are bounded by the CLI git-buffer limit and fail rather than silently truncating the raw fingerprint input.
108116
- Binary files are recorded but do not generate identifier-based queries.
109117
- The packet is context, not a verdict. A consumer should never turn `preflight.ready` into "the change is correct."

0 commit comments

Comments
 (0)