feat: include human replies in review context #17

Merged

nimmo merged 6 commits from issue-16-human-review-context into main 2026-07-20 21:18:52 +01:00

Conversation 1 Commits 6 Files changed 5 +103 -8

nimmo commented 2026-07-20 20:48:56 +01:00

Owner

Copy link

Implements #16.\n\nMakes bounded, post-review human replies available as explicitly untrusted context on subsequent automated review runs, while excluding the previous automated review itself.\n\nCloses #16.

Implements #16.\n\nMakes bounded, post-review human replies available as explicitly untrusted context on subsequent automated review runs, while excluding the previous automated review itself.\n\nCloses #16.

nimmo added 1 commit 2026-07-20 20:48:57 +01:00

feat: include human replies in review context 1f35137986

nimmo added 1 commit 2026-07-20 20:54:36 +01:00

fix: parse commented YAML list keys 7aea339375

nimmo added 1 commit 2026-07-20 20:54:59 +01:00

ci: advance self-review action revision 0d31c9f469

nimmo added 2 commits 2026-07-20 20:57:32 +01:00

fix: strip YAML inline comments 12b3315d00

ci: pin self-review to YAML parser fix 6bbc7afda7

nimmo commented 2026-07-20 21:00:59 +01:00

Author

Owner

Copy link

Automated code review

Reviewed commit: 533617d4e42aa9141f8aa3e87a06091358a7b2b8

Verdict: Ready to merge

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

Overall assessment

The pull request implements bounded, post-review human context injection with explicit untrusted labeling, byte-size limits, and AI output exclusion. The implementation correctly parses YAML comments, safely truncates UTF-8 text, and integrates the context into both review and validation prompts. Tests cover edge cases for parsing, truncation, and filtering.

Blocking findings

None.

Important findings

None.

Suggestions

• YAML single-quote escape handling omitted in comment stripper (reviewer.py:148; high confidence)
  elif character in ("'", '"'):
  quote = character
  Impact: Values containing YAML single-quote escapes (e.g., 'it''s fine' # comment) may be incorrectly truncated or parsed as comments.
  Suggested fix: Add a check for consecutive single quotes to toggle the escape state, or document that single-quoted scalars with escapes are out of scope.

• Timestamp comparison relies on lexicographical ordering (reviewer.py:235; medium confidence)
  and comment.get("created_at", "") > after
  Impact: Works for standard ISO 8601 timestamps but could fail or produce unexpected ordering if non-standard formats or missing timezone indicators appear in the API response.
  Suggested fix: Parse timestamps with datetime.fromisoformat() for robust comparison, or explicitly document the expected timestamp format requirement.

Tests and validation

• unit-tests: success

Questions

• Does the repository anticipate config values containing YAML single-quote escapes ('') that would require updating strip_yaml_comment?
• How will the system behave if max_reply_bytes is set to a value smaller than the byte size of a single human reply? The current truncation logic will drop the entire reply, which may be intended but could benefit from explicit documentation.

Review limitations

• Review is advisory and based solely on the supplied diff. No runtime execution, dependency analysis, or external security scanning was performed.
• Changes to unchanged files outside the provided diff were not evaluated.

Diff coverage

• .forgejo/ai-review.yml: reviewed — included in a context-limited batch
• .forgejo/workflows/ai-review.yml: reviewed — included in a context-limited batch
• README.md: reviewed — included in a context-limited batch
• reviewer.py: reviewed — included in a context-limited batch
• tests/test_reviewer.py: reviewed — included in a context-limited batch

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: 2

<!-- forgejo-ai-review:v1 --> ## Automated code review Reviewed commit: `533617d4e42aa9141f8aa3e87a06091358a7b2b8` ## Verdict: Ready to merge No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory. ### Overall assessment The pull request implements bounded, post-review human context injection with explicit untrusted labeling, byte-size limits, and AI output exclusion. The implementation correctly parses YAML comments, safely truncates UTF-8 text, and integrates the context into both review and validation prompts. Tests cover edge cases for parsing, truncation, and filtering. ### Blocking findings None. ### Important findings None. ### Suggestions - **YAML single-quote escape handling omitted in comment stripper** (reviewer.py:148; high confidence) elif character in ("'", '"'): quote = character Impact: Values containing YAML single-quote escapes (e.g., 'it''s fine' # comment) may be incorrectly truncated or parsed as comments. Suggested fix: Add a check for consecutive single quotes to toggle the escape state, or document that single-quoted scalars with escapes are out of scope. - **Timestamp comparison relies on lexicographical ordering** (reviewer.py:235; medium confidence) and comment.get("created_at", "") > after Impact: Works for standard ISO 8601 timestamps but could fail or produce unexpected ordering if non-standard formats or missing timezone indicators appear in the API response. Suggested fix: Parse timestamps with datetime.fromisoformat() for robust comparison, or explicitly document the expected timestamp format requirement. ### Tests and validation - unit-tests: **success** ### Questions - Does the repository anticipate config values containing YAML single-quote escapes ('') that would require updating strip_yaml_comment? - How will the system behave if max_reply_bytes is set to a value smaller than the byte size of a single human reply? The current truncation logic will drop the entire reply, which may be intended but could benefit from explicit documentation. ### Review limitations - Review is advisory and based solely on the supplied diff. No runtime execution, dependency analysis, or external security scanning was performed. - Changes to unchanged files outside the provided diff were not evaluated. ### Diff coverage - `.forgejo/ai-review.yml`: **reviewed** — included in a context-limited batch - `.forgejo/workflows/ai-review.yml`: **reviewed** — included in a context-limited batch - `README.md`: **reviewed** — included in a context-limited batch - `reviewer.py`: **reviewed** — included in a context-limited batch - `tests/test_reviewer.py`: **reviewed** — included in a context-limited batch <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: 2 </details>

nimmo added 1 commit 2026-07-20 21:03:47 +01:00

fix: harden untrusted reply context 533617d4e4

nimmo merged commit 84b8aaeed3 into main 2026-07-20 21:18:52 +01:00

nimmo referenced this pull request from a commit 2026-07-20 21:18:52 +01:00

feat: include human replies in review context (#17)

nimmo deleted branch issue-16-human-review-context 2026-07-20 21:18:52 +01:00

nimmo added this to the Forgejo AI PR Review v1 milestone 2026-07-20 22:00:22 +01:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Forgejo AI PR Review v1

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

nimmo/forgejo-auto-pr-review!17

No description provided.