-
Notifications
You must be signed in to change notification settings - Fork 13
feat(review): add bounded review-context packets #128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f1b3bd5
d5a6182
230e204
da4ea27
f26a613
e728e93
04cd606
d52db63
f5513bb
794432c
ea33b7c
31f6ae9
3174b1d
bc962fc
00730aa
37f96fc
5a9a5d9
2f6bca0
cb5ade8
bafe338
d05c4b2
9544365
c30a245
d65e535
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| # Review context | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This commit introduces a new Markdown document without archiving or deleting another Markdown file, directly violating the repository's documentation constraint. Update an existing document or archive one before adding this file. AGENTS.md reference: AGENTS.md:L206-L211 Useful? React with 👍 / 👎. |
||
|
|
||
| `codebase-context-review` turns a committed git diff into a bounded review-context packet. | ||
|
|
||
| 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. | ||
|
|
||
| ## Why this exists | ||
|
|
||
| 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: | ||
|
|
||
| 1. **context compilation** — git + local repository analysis | ||
| 2. **review reasoning** — any model or human reviewer consuming the packet | ||
| 3. **evaluation** — measuring whether the context actually helps find the right files/spans/issues | ||
|
|
||
| Keeping those layers separate makes failures diagnosable. A bad review can be attributed to missing context, bad reasoning, or both instead of hiding every variable behind one agent loop. | ||
|
|
||
| ## Usage | ||
|
|
||
| Build and run from the repository: | ||
|
|
||
| ```bash | ||
| pnpm build | ||
| node dist/review-bin.js --base origin/main --head HEAD | ||
| ``` | ||
|
|
||
| After package publication, the package also exposes: | ||
|
|
||
| ```bash | ||
| npx codebase-context-review --base origin/main --head HEAD | ||
| ``` | ||
|
|
||
| Use `--json` for the complete machine-readable packet: | ||
|
|
||
| ```bash | ||
| npx codebase-context-review \ | ||
| --base origin/main \ | ||
| --head HEAD \ | ||
| --max-queries 8 \ | ||
| --max-results 3 \ | ||
| --json > review-context.json | ||
| ``` | ||
|
|
||
| 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? | ||
|
|
||
| `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. | ||
|
|
||
| ## Packet contract | ||
|
|
||
| The current schema is `review-context-v1`. | ||
|
|
||
| The packet contains: | ||
|
|
||
| - exact resolved base/head commit SHAs | ||
| - SHA-256 fingerprint of Git's exact raw diff output | ||
| - changed file status, additions/deletions, rename source, and binary flag | ||
| - NUL-safe filename parsing for unusual valid git paths | ||
| - identifiers extracted deterministically from changed lines | ||
| - bounded search queries derived from those diff signals | ||
| - bounded related-context results from the existing `search_codebase` engine | ||
| - edit-preflight/search-quality metadata when the search engine provides it | ||
| - current team-pattern/convention output | ||
| - explicit warnings when context could not be produced | ||
|
|
||
| Absolute local repository paths are not part of the packet contract. | ||
|
|
||
| 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. | ||
|
|
||
| ## Bounds | ||
|
|
||
| Defaults: | ||
|
|
||
| | Bound | Default | | ||
| | --- | ---: | | ||
| | Changed-file search queries | 8 | | ||
| | Related results per query | 3 | | ||
| | Identifier candidates per file | 10 | | ||
| | Snippet characters per related result | 1,200 | | ||
|
|
||
| These are explicit because an unbounded context compiler is not useful evidence. Increasing them is allowed, but benchmark runs should record the values and compare like-for-like. | ||
|
|
||
| ## Index behavior | ||
|
|
||
| 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`. | ||
|
|
||
| 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. | ||
|
|
||
| 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. | ||
|
|
||
| ## What this proves | ||
|
|
||
| Shipping this command proves only that the project can compile a bounded, versioned review-oriented context packet from an exact committed git range. | ||
|
|
||
| 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. | ||
|
|
||
| ## Evaluation path | ||
|
|
||
| The intended evaluation is an ablation, not a marketing benchmark: | ||
|
|
||
| 1. freeze a set of public PR/bug-fix tasks before observing lane outputs | ||
| 2. run the same reviewer/model with **raw diff only** | ||
| 3. run the same reviewer/model with **raw diff + `review-context-v1` packet** | ||
| 4. keep model, prompt, turn/token budget, timeout, and scoring fixed | ||
| 5. score bug/finding recall and precision, evidence quality, false positives, token cost, and wall time separately | ||
| 6. report setup/index failures as failures, not as competitor losses | ||
| 7. do not publish a superiority claim from a one-task pilot | ||
|
|
||
| The existing ContextBench protocol already follows the same evidence discipline for repository-context retrieval. Review-specific claims should meet at least the same standard rather than creating a weaker parallel benchmark. | ||
|
|
||
| ## Current limitations | ||
|
|
||
| - Only committed git refs are supported in v1; working-tree/staged review is intentionally deferred. | ||
| - `--head` must be the checked-out commit and the worktree must be clean. | ||
| - Query generation is lexical and deterministic. It extracts identifiers from changed lines and falls back to path signals; it is not AST-aware yet. | ||
| - `--no-index` deliberately skips freshness work, so benchmark callers must attest the index/source match themselves. | ||
| - Large diffs are bounded by the CLI git-buffer limit and fail rather than silently truncating the raw fingerprint input. | ||
| - Binary files are recorded but do not generate identifier-based queries. | ||
| - The packet is context, not a verdict. A consumer should never turn `preflight.ready` into "the change is correct." | ||
|
|
||
| ## Next gate | ||
|
|
||
| Do not add reviewer-agent features until a small frozen public evaluation can answer this first question: | ||
|
|
||
| > Does `review-context-v1` improve relevant-file/span coverage at an acceptable precision and context cost compared with the same reviewer using the raw diff/repository tools alone? | ||
|
|
||
| If the answer is no, fix or kill this lane before adding more orchestration. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,7 +44,8 @@ | |
| } | ||
| }, | ||
| "bin": { | ||
| "codebase-context": "./dist/index.js" | ||
| "codebase-context": "./dist/index.js", | ||
| "codebase-context-review": "./dist/review-bin.js" | ||
| }, | ||
| "files": [ | ||
| "dist", | ||
|
|
@@ -54,6 +55,7 @@ | |
| "docs/cli.md", | ||
| "docs/capabilities.md", | ||
| "docs/client-setup.md", | ||
| "docs/review-context.md", | ||
| "templates" | ||
| ], | ||
| "packageManager": "[email protected]", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| import { runReviewCli } from './review-cli.js'; | ||
|
|
||
| runReviewCli(process.argv.slice(2)).catch((error) => { | ||
| console.error(error instanceof Error ? error.message : String(error)); | ||
| process.exitCode = 1; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change adds another Markdown document without deleting or updating an existing Markdown document to make room, increasing the documentation set contrary to the repository's explicit maintenance constraint.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!