feat: predictable large pull-request coverage

nimmo commented

2026-07-20 20:12:27 +01:00

Owner

Implements #7.\n\nReplaces first-byte diff truncation with deterministic file batches, configurable exclusions, and a named coverage report.\n\nCloses #7.

nimmo added 1 commit

2026-07-20 20:12:27 +01:00

feat: batch large pull request reviews

AI pull-request review / review (pull_request) Successful in 3m53s

Details

7a361f13ba

nimmo commented

2026-07-20 20:16:22 +01:00

Author

Owner

Automated code review

Reviewed commit: cfb65718c775f4fe6bf8d307edeb94f8d0213a95

Verdict: Ready to merge

No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory.

Overall assessment

The pull request implements deterministic batching, configurable path exclusions, and explicit coverage reporting for large diffs, replacing the previous first-byte truncation approach. The changes correctly spool diff sections to temporary files to bound memory usage, enforce byte limits per batch, and provide a named coverage report in the final comment. Test coverage is expanded to validate spooling, batching logic, exclusion filtering, and byte-boundary handling.

Blocking findings

None.

Important findings

None.

Suggestions

Byte-boundary truncation silently drops characters (reviewer.py:318; medium confidence)
head = section.stream.read(head_size).decode(errors="ignore")
Impact: Splitting a binary stream at an arbitrary byte boundary and decoding with errors="ignore" silently drops incomplete UTF-8 sequences. While acceptable for LLM prompts, it reduces deterministic fidelity and may obscure context near the truncation point.
Suggested fix: Use errors="replace" to preserve character count visibility, or truncate at valid UTF-8 boundaries using a codec-aware splitter before decoding.

Tests and validation

unit-tests: success

Questions

Does the Forgejo API guarantee that diff --git headers always follow the standard quoting format, or should the reviewer handle non-standard metadata lines more gracefully?
Should the coverage report explicitly list files that were skipped due to binary content versus those omitted by exclude_paths, or is the current status grouping sufficient?

Review limitations

Analysis is based solely on the provided diff and documentation; runtime behavior depends on Forgejo's exact diff formatting and Ollama endpoint availability.
Path extraction logic assumes standard Git diff header structures; edge cases with heavily customized or non-standard diff outputs may not be fully covered by tests.

Review metadata

Reviewer: forgejo-ai-review v1
Model: qwen3.6:35b-a3b-q8_0
Guidance loaded: .forgejo/ai-review.yml, README.md
Commits considered: 6
Previous automated review: updated
Findings: 1

## Automated code review Reviewed commit: `cfb65718c775f4fe6bf8d307edeb94f8d0213a95` ## Verdict: Ready to merge No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory. ### Overall assessment The pull request implements deterministic batching, configurable path exclusions, and explicit coverage reporting for large diffs, replacing the previous first-byte truncation approach. The changes correctly spool diff sections to temporary files to bound memory usage, enforce byte limits per batch, and provide a named coverage report in the final comment. Test coverage is expanded to validate spooling, batching logic, exclusion filtering, and byte-boundary handling. ### Blocking findings None. ### Important findings None. ### Suggestions - **Byte-boundary truncation silently drops characters** (reviewer.py:318; medium confidence) head = section.stream.read(head_size).decode(errors="ignore") Impact: Splitting a binary stream at an arbitrary byte boundary and decoding with `errors="ignore"` silently drops incomplete UTF-8 sequences. While acceptable for LLM prompts, it reduces deterministic fidelity and may obscure context near the truncation point. Suggested fix: Use `errors="replace"` to preserve character count visibility, or truncate at valid UTF-8 boundaries using a codec-aware splitter before decoding. ### Tests and validation - unit-tests: **success** ### Questions - Does the Forgejo API guarantee that `diff --git` headers always follow the standard quoting format, or should the reviewer handle non-standard metadata lines more gracefully? - Should the coverage report explicitly list files that were skipped due to binary content versus those omitted by `exclude_paths`, or is the current status grouping sufficient? ### Review limitations - Analysis is based solely on the provided diff and documentation; runtime behavior depends on Forgejo's exact diff formatting and Ollama endpoint availability. - Path extraction logic assumes standard Git diff header structures; edge cases with heavily customized or non-standard diff outputs may not be fully covered by tests. <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3.6:35b-a3b-q8_0 - Guidance loaded: .forgejo/ai-review.yml, README.md - Commits considered: 6 - Previous automated review: updated - Findings: 1 </details>

nimmo added 1 commit

2026-07-20 20:19:25 +01:00

fix: spool large diff sections

AI pull-request review / review (pull_request) Successful in 4m15s

Details

a16bb28fd8

nimmo added 1 commit

2026-07-20 20:25:04 +01:00

docs: clarify large review coverage

AI pull-request review / review (pull_request) Successful in 4m24s

Details

befcc7c18c