alibaba / alibaba/open-code-review
Preview omits diff-level vendor directory drops from file totals and exclusion reasons; no tracked-vendor override in v1.11.6
- Dominant language
- Go
- Stars
- 24.4k
- Forks
- 1.8k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 105
Description
## Observed
With OCR v1.11.6, installed build `04284b5dc` matching tag commit `04284b5dc19e0f52528ca466a767e4a2c100e5f5`, an explicit Git range containing24 changed tracked files produces a successful preview listing17 files with `excluded_count: 0`. The other7 are tracked Rust source/tests under `vendor/`, not binaries or Git-ignored files. Other tested ranges reconcile34/34,13/13 and33/33.
Command shape:
```sh
ocr review --preview --from --to \
--rule /absolute/full-scope.json --repo /absolute/repository \
--provider zai-anthropic --model glm-5.3 --format json
```
The rule has a `rules` entry matching `**/*`, top-level `include: ["**/*"]` and `exclude: []`. These are supported schema fields, and the include list is active in preview. No LLM review was run.
This arose while preparing review of intentionally modified vendored code in `vybestack/llxprt-code-rs`, with24 files in the exact local range `2f0af9c4098c3672eb323363a6f0df62e689aa00` to `28e2399eb21b9c4820c31f635ddc04deece47fdc`. Local feature commits may not be remotely available. The seven omitted files are:
- `vendor/serdes-ai-models/src/anthropic/model.rs`
- `vendor/serdes-ai-models/src/anthropic/stream.rs`
- `vendor/serdes-ai-models/src/anthropic/types.rs`
- `vendor/serdes-ai-responses/src/client/assembler.rs`
- `vendor/serdes-ai-responses/src/client/mod.rs`
- `vendor/serdes-ai-responses/src/types.rs`
- `vendor/serdes-ai-responses/tests/client_ws.rs`
## Source-qualified cause and distinction
At exact v1.11.6, `internal/diff/git.go:27–40` defines `providerDirIgnoreDirs`, including `vendor/`; `filterDiffs` at387–400 removes these paths before agent file selection. `internal/agent/preview.go` therefore never classifies them or includes them in excluded counts. `ocr delegate` uses the same selection counts.
The early filtering is documented in `pages/src/content/docs/en/review-rules.md:126–128`. This report is not claiming an undocumented implementation accident or malformed rule parsing. However, the FAQ at `faq.md:96–102` promises a reason for every candidate and illustrates `node_modules/lib.js` as `default_path`, even though that directory is removed earlier in this version. Preview totals cannot be reconciled with the Git range without a separate diff inventory.
Source/help inspection found no supported flag, environment or rule override for `providerDirIgnoreDirs`. The include filter can bypass later per-file gates, but cannot restore paths already removed upstream. This leaves a separate feature gap for repositories that intentionally modify tracked vendor code.
## Requested behavior
1. Account for diff-level drops with explicit paths/reasons, or a separate dropped-file count, so preview/delegate totals reconcile with Git and do not suggest no exclusions occurred.
2. Correct the FAQ example and explain which exclusions cannot be overridden.
3. Provide an explicit supported way to include tracked vendored changes when full review requires them.
No tool patch, path-renamed repository or private filtering workaround was applied. Existing raw command, preview JSON, Git inventories and exact-version source diagnosis are retained locally under `branch-4/llxprt-code-rs/evalwork/results/branch4-wave2/ocr-coverage-preflight/`.
Contributor guide
Research direction
Start in internal/diff/git.go at providerDirIgnoreDirs and filterDiffs, then trace how internal/agent/preview.go reports selection and excluded counts. Reproduce the supplied ocr review --preview command against the exact Git range, and inspect review-rules.md and faq.md. Done means dropped tracked vendor paths are accounted for, the FAQ explains non-overridable exclusions, and an explicit supported override exists.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- documentation, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100