feat(review): add bounded review-context packets - #128
Conversation
Greptile SummaryAdds a new CLI that turns a committed Git range into a bounded review-context packet, together with packet assembly utilities, tests, package wiring, and documentation.
Confidence Score: 3/5The pull request should not merge until packet construction is pinned to resolved commits and changed-file parsing preserves valid Git filenames. Mutable refs can make the packet's recorded commits disagree with its diff-derived content, while line-oriented name-status parsing can omit or misattribute files with Git-special names. Files Needing Attention: src/review-cli.ts, src/review-context.ts, docs/review-context.md
|
| Filename | Overview |
|---|---|
| src/review-cli.ts | Adds the executable workflow and local indexing/tool context, but repeated diffs use mutable refs rather than the resolved commit SHAs. |
| src/review-context.ts | Adds deterministic packet assembly, identifier ranking, bounds, and result compaction, but name-status parsing does not preserve all valid Git paths. |
| tests/review-context.test.ts | Covers core parsing, ranking, bounding, fingerprinting, and packet assembly, but not mutable refs or Git-special filenames. |
| docs/review-context.md | Documents the new workflow and appropriately limits its claims, but introduces a new Markdown file contrary to repository documentation governance. |
| package.json | Publishes the new executable and documentation file through the package manifest. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Base and head refs] --> B[Resolve commit SHAs]
A --> C[Collect Git range and per-file diffs]
C --> D[Parse changed files and identifiers]
D --> E[Build bounded search queries]
E --> F[Search local index]
G[Team pattern lookup] --> H[Review-context packet]
B --> H
C --> H
D --> H
F --> H
Reviews (1): Last reviewed commit: "docs(review): document review context co..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const rootPath = path.resolve(gitRoot); | ||
| const baseCommit = resolveCommit(rootPath, options.base); | ||
| const headCommit = resolveCommit(rootPath, options.head); | ||
| const range = `${options.base}...${options.head}`; |
There was a problem hiding this comment.
When the base or head ref moves during packet construction, the separate Git commands resolve different commits while refs.baseCommit and refs.headCommit retain the earlier SHAs, causing the fingerprint, changed-file data, and per-file patches to describe a different or internally inconsistent range.
| const range = `${options.base}...${options.head}`; | |
| const range = `${baseCommit}...${headCommit}`; |
| head: string; | ||
| baseCommit: string; | ||
| headCommit: string; | ||
| rawDiff: string; | ||
| changedFiles: ChangedFileDescriptor[]; |
There was a problem hiding this comment.
When a committed filename contains tabs, newlines, significant surrounding whitespace, or characters represented through Git quoting, trimming and splitting non-NUL --name-status output changes or splits the path; the subsequent per-file diff then omits or misattributes that file even though it remains included in the raw-diff fingerprint.
| @@ -0,0 +1,117 @@ | |||
| # Review context | |||
There was a problem hiding this comment.
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!
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e728e938b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '--find-renames', | ||
| range, | ||
| '--', | ||
| file.path |
There was a problem hiding this comment.
For every renamed file, this pathspec includes only the destination path. I reproduced this in a temporary repository: although git diff -h documents -M as detecting renames, git diff --find-renames base...head -- new.txt rendered a 100% rename as a newly added file. Consequently, rename-only changes report every unchanged line as an addition and extract identifiers from unchanged content, corrupting packet statistics and evaluation signals; include previousPath as well or split the unfiltered diff.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,117 @@ | |||
| # Review context | |||
There was a problem hiding this comment.
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 👍 / 👎.
| return parsed; | ||
| } | ||
|
|
||
| function parseArgs(argv: string[]): ReviewCliOptions | undefined { |
There was a problem hiding this comment.
This starts a separate CLI implementation containing argument parsing, Git orchestration, indexing, and output formatting, while the repository explicitly requires CLI code to live in src/cli.ts. Move this implementation there, leaving at most a thin executable shim for the additional binary.
AGENTS.md reference: AGENTS.md:L226-L231
Useful? React with 👍 / 👎.
| parseToolJson<SearchResponse>( | ||
| await dispatchTool( | ||
| 'search_codebase', | ||
| { query, includeSnippets: true, intent: 'edit', limit }, |
There was a problem hiding this comment.
When consumers request the machine-readable packet, snippets are always enabled and the defaults can include up to 24 snippets of 1,200 characters each—many thousands of tokens before other metadata—with no summary-only option. Default to summaries and paths or require an explicit snippet flag so review output respects the repository's response-budget and default-output requirements.
AGENTS.md reference: AGENTS.md:L220-L224
Useful? React with 👍 / 👎.
| ), | ||
| loadConventions: async () => | ||
| parseToolJson<PatternResponse>( | ||
| await dispatchTool('get_team_patterns', { category: 'all' }, toolContext), |
There was a problem hiding this comment.
In repositories with many stored team memories, get_team_patterns with category: 'all' returns every matching memory and this response is copied wholesale into the packet without a count or character limit. Thus the advertised bounded packet can grow arbitrarily even when query, result, and snippet limits are respected; compact or cap this convention payload and report truncation.
AGENTS.md reference: AGENTS.md:L220-L224
Useful? React with 👍 / 👎.
| throw new Error(`${operation} returned invalid JSON`); | ||
| } | ||
|
|
||
| if (response.isError) { |
There was a problem hiding this comment.
When intelligence.json is missing or corrupt, get_team_patterns returns a JSON {status: 'error', ...} payload without setting isError; this check therefore accepts it as a valid PatternResponse, and the packet stores the error object under conventions without adding a warning. Validate the operation's status/schema so failed convention retrieval is explicitly reported rather than presented as context.
AGENTS.md reference: AGENTS.md:L191-L194
Useful? React with 👍 / 👎.
| for (const rawLine of output.split(/\r?\n/)) { | ||
| const line = rawLine.trim(); | ||
| if (!line) continue; | ||
|
|
||
| const parts = line.split('\t'); |
There was a problem hiding this comment.
For valid repository paths containing tabs, newlines, or trailing spaces, newline/tab parsing either receives Git's quoted pseudo-path or strips part of the filename with trim(), after which the per-file diff lookup targets a nonexistent path and produces an empty patch. git diff -h documents -z as producing NUL-terminated diff-raw output; invoke --name-status -z and parse NUL-separated fields to preserve filenames exactly.
Useful? React with 👍 / 👎.
PatrickSys
changed the title
1 participant
What
Adds the first review-oriented vertical slice to
codebase-contextwithout importing or relying on the abandoned privatecontext-kitimplementation.codebase-context-reviewbinaryreview-context-v1packetsearch_codebaseretrieval + edit preflight metadata--no-indexfor controlled benchmark runsdocs/review-context.mdReproducibility boundary
The git envelope, identifier extraction, bounds, and query derivation are deterministic. Related-context ranking is reproducible only under a frozen
codebase-contextversion, index, embedding/reranking configuration, and runtime dependencies. The docs state that explicitly rather than treating semantic retrieval as environment-independent.Non-claims
This does not claim better review quality. It does not call an LLM or post comments. The packet is a context input so raw-diff vs enriched-context reviewer runs can be evaluated without mixing retrieval and reasoning.
The next gate is evidence: freeze a small public review task set and compare the same reviewer/model under fixed budgets before adding reviewer-agent orchestration.
Safety / scope
context-kithistory copied into this branch