Extract utility functions (stripEmptyPlanBlock, stripMarkdownFences,
buildMessageXML, copyMessages, countMessagesTokens, reviewModeString,
detectGitBranch) into util.go and all compression logic into compression.go
to improve readability (agent.go: 1489 → 1114 lines).
Fix pre-existing compression bugs from commit 14adf6f:
- Return unmodified messages on compression failure instead of truncating
to frozen zone (which discarded all conversation context)
- Check error in async compression goroutine to avoid applying failed results
- Track snapshot length to append post-snapshot messages when applying async
compression, preventing silent message loss
- Strip old <previous_review_summary> before appending new one to prevent
cumulative frozen zone bloat across multiple compressions
- Guard pendingJob cleanup to only clear when it matches the completed job,
preventing accidental clearing of a newly scheduled job
- Add context.Context and 5s timeout to detectGitBranch
- Deep copy ToolCalls in copyMessages to match session/history.go
- Reuse finalCount in addNextMessage to avoid redundant token counting
When a file was renamed on the target branch, ocr review emitted
'[ocr] WARNING: cannot read file <old path> at ref <to>: exit status 128'.
Two compounding bugs:
1. The parser required 'a/'/'b/' prefixes when matching '--- /dev/null' /
'+++ /dev/null', but git emits these lines without prefixes, so
IsNew/IsDeleted were never set and deleted files fell through to a
doomed 'git show ref:<old path>'.
2. 'rename from' / 'rename to' extended headers were never parsed, and
git diff/show call sites did not force rename detection, so renames
degraded to delete+add whenever the user had diff.renames=false.
Fixes:
- Parse 'rename from'/'rename to', 'new file mode', 'deleted file mode'
and unprefixed /dev/null markers in ParseDiffText.
- Pass --find-renames to all git diff/show invocations so rename
detection no longer depends on user config.
- Add IsRenamed to model.Diff (json: is_renamed) and prefer it in
diffStatus.
- Add parser unit tests and a range-mode rename regression test.
Fixes#99
- Add REVIEW_FILTER_TASK template and LLM conversation config
- Implement executeReviewFilter to post-process comments per file
- Add CommentsForPath and RemoveByPathAndIndices to CommentCollector
- Apply timeout from template config to review filter LLM call
- Await CommentWorkerPool before running filter to prevent race condition
- Include raw response preview in filter JSON parse error logs
Introduce gitcmd.Runner with channel-based semaphore to cap concurrent
git subprocesses (default 16, configurable via --max-git-procs). Route
all tool-layer and diff-layer git calls through the shared runner.
Also fix diff.Provider.runGit lacking context.Context — now the full
chain (GetDiff → MergeBase → ParseDiffText → finalizeDiff) propagates
the caller's context for proper cancellation and timeout support.
Registry was a plain map alias and DiffMap a shared map[string]string,
both relying on implicit write-before-read ordering with no compile-time
safety. Refactor Registry into an encapsulated struct with Freeze() that
panics on post-freeze writes, and wrap DiffMap as an immutable value type
with defensive copy on construction. Move DiffMap injection to before
filterDiffs to preserve the original behavior of exposing all file diffs.
Add cache_control ephemeral breakpoints on system prompt and tool
definitions to activate Anthropic prompt caching for multi-turn
agent loops. Fix token double-counting by removing the redundant
totalTokensUsed accumulator and computing it from sub-items. Surface
cache read/write statistics in both text and JSON summary output.
Add ctx parameter to Provider.Execute interface and propagate it through
FileReader.Read, code_search.gitGrep/runGitGrep, and file_find.listGitFiles.
This ensures parent task cancellation/timeout properly terminates running
git subprocesses (grep, show, ls-files, ls-tree). Also adds 10s timeout
to file_find and fixes context.Canceled handling in code_search.
StreamCompletion used http.NewRequest without context and retry with
context.Background(), making streaming calls uncancellable. Memory
compression called Completions() without context, leaving async
goroutines unable to be cancelled or timed out.
- Add StreamCompletionWithCtx to LLMClient interface and both
OpenAI/Anthropic implementations
- Propagate context through runCompression, addNextMessage, and
triggerAsyncCompression call chain
- Add 5-minute timeout cap on async compression goroutines
- Cancel in-flight compression HTTP requests when job is superseded
Store ReviewMode, DiffFrom, DiffTo, and DiffCommit in SessionHistory
and persist them to JSONL (conditionally, only when non-empty). Display
mode in viewer list page and show version details (from/to or commit)
on the session detail page based on review mode.
code_search used SplitN(":", 3) to parse git grep output, but in
range/commit mode git grep prefixes each line with the ref
(ref:file:line:content). All matches silently failed Atoi on the
filepath field, returning an empty string that caused the review
session to terminate immediately.
Fix code_search to detect the ref prefix and adjust split/index
accordingly. Also replace the hard stop on empty tool results with
a retry loop (up to 3 consecutive empty rounds) so the LLM can
recover or call task_done gracefully.
Replace the hard-coded Chinese-only empty-plan branch with a compiled
regex + helper that strips the "### Review Plan …" wrapper (any language
variant) when the plan phase produces no output.
TWO BUGS fixed:
- The wrapper was only stripped for the Chinese literal `### 审查计划`,
not the actual English `### Review Plan (Optional)` shipped in the
template, so `{{plan_guidance}}` leaked into the rendered prompt on
every review where the plan phase was skipped or failed.
- The strip ran AFTER `ReplaceAll("{{plan_guidance}}", "")`, so the
token was already gone and the regex couldn't match the wrapper.
Fixed ordering: strip BEFORE replace.
Also simplifies the regex alternation per Copilot review feedback:
`Review Plan` and `审查计划` already subsume the `(Optional)`/`（可选）`
suffixes because the trailing `[^\n]*` consumes them.
Adds unit tests for English, Chinese, and integration (production flow).
Co-authored-by: Copilot <copilot-pull-request-reviewer@github.com>
* fix(agent): skip binary files during code review
Previously, binary files were passed through the review pipeline because
shouldReview only checked path/extension rules. This caused the agent to
incorrectly attempt reviewing binary content.
- Add ExcludeBinary reason to the exclusion filter
- Change shouldReview/whyExcluded to accept model.Diff instead of path
string, enabling access to the IsBinary flag
- Check IsBinary as the first gate in whyExcluded
- Differentiate skip log messages for binary vs path-filtered files
* test: add unit tests for preview.go
* fix: normalize dash to em-dash in binary file skip log message
* test(agent): strengthen UserIncludePattern tests to cover additive semantics
- Document that include patterns are additive, not exclusive
- Add cases verifying include bypasses default-path exclusion
- Add cases for extension filter precedence over include logic
- Add TestWhyExcluded_IncludeBypassesDefaultPath for rescue semantics
- Add TestWhyExcluded_IncludeAndExcludeInteraction for priority order
Re-location LLM calls were invisible in session history and their token
consumption was unaccounted for. Refactor the code_comment handling path
to record re-location requests/responses into session JSONL, accumulate
token usage into global counters, and display them in the viewer.
Also fixes: async context cancellation risk (WithoutCancel), duplicate
telemetry recording, missing timeout enforcement, missing {existing_code}
placeholder in the re-location prompt, and renames Parse to ParseComments
for clarity.
Allow users to configure include/exclude glob patterns in
.open-code-review/rule.json (global or project-level) to control
which files are reviewed. Exclude has highest priority, include
penetrates default path exclusions but not the extension allowlist.
Only one layer takes effect following --rule > project > global priority.
Consolidate review rules into a composedResolver that supports four layers
(--rule flag > project .open-code-review/rule.json > global ~/rule.json >
embedded system default), each with first-match-wins fall-through semantics.
This change introduces a new configuration mechanism for excluding specific file paths during code reviews. It adds support for glob patterns including recursive directory matching (**), single-segment wildcards (*), and brace expansion ({a,b}), allowing more flexible control over which files should be reviewed or skipped based on their location within the repository.
This change refines how file counts are tracked during the agent's review process. It now correctly excludes deleted files when calculating the number of files being reviewed, ensuring telemetry and error reporting reflect actual processing rather than total diffs. This improves accuracy in monitoring and debugging scenarios.
When all subtask LLM calls fail (e.g. invalid API key), dispatchSubtasks
now returns an error instead of empty comments with nil error. Partial
failures are recorded as subtask_error warnings so output functions can
distinguish "no issues found" from "review failed".
kite