test: exercise AI review deduplication - #3300
Conversation
|
/review inline |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3300 +/- ##
==========================================
- Coverage 90.35% 90.32% -0.04%
==========================================
Files 251 250 -1
Lines 89404 89020 -384
Branches 89404 89020 -384
==========================================
- Hits 80784 80404 -380
+ Misses 5685 5682 -3
+ Partials 2935 2934 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
This PR adds a single uncompiled fixture file, kernel/src/ai_review_experiment.rs, for the review-workflow experiment in #3299. The reviewers flagged the two intended targets: first_retry_delay indexes delays[0] and panics on an empty slice, and parse_retry_count calls value.parse().unwrap() and panics on invalid input. The disprove gate dropped both as blockers because the file is not declared as a module, has no callers, and is explicitly a temporary do-not-merge fixture, so neither panic path is reachable. The same reasoning removed the orphaned-file concern. Nothing here is a blocking defect given the stated intent.
Non-blocking notes
Nit1 - kernel/src/ai_review_experiment.rs:7
The comment // TODO: define whether an empty retry plan is valid. is an untracked TODO with no issue reference, unlike the TODO(#3299) marker below it. If any of this logic is ever promoted out of the fixture, link it to an issue or resolve the empty-plan contract.
Raised by: docs-reviewer, maintainer-claude-reviewer
Suggested fix: attach an issue reference, for example TODO(#NNNN): ..., or resolve the empty-retry-plan semantics.
Summary
No blocking issues. The two panic paths and the source-root placement are intentional review targets in an uncompiled, do-not-merge fixture, and the disprove gate confirmed none is a reachable defect in this PR; if this code is ever productionized, switch first_retry_delay to a fallible return, replace the unwrap in parse_retry_count, and resolve the untracked TODO. All five primary reviewers and the disprove gate completed, so coverage was full.
Automated review - workflow run
| delays[0] | ||
| } | ||
|
|
||
| // TODO: define whether an empty retry plan is valid. |
There was a problem hiding this comment.
Nit1 This is an untracked TODO with no issue reference, unlike the TODO(#3299) marker below it. If this logic is ever promoted out of the fixture, link it to an issue or resolve the empty-retry-plan contract. Raised by: docs-reviewer, maintainer-claude-reviewer. Suggested fix: attach an issue reference such as TODO(#NNNN) or resolve the empty-plan semantics.
|
/review inline |
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds a single new file, kernel/src/ai_review_experiment.rs, that is never declared with mod ai_review_experiment; anywhere in the crate. It is not part of the compilation graph, so nothing in it is compiled, reachable, or callable, and its pub functions add no public API surface. The panic-shaped patterns in it (delays[0] on a possibly empty slice, value.parse().unwrap() on arbitrary input) cannot be triggered through the crate as written, so they are not present-in-the-diff correctness defects. The // TODO(#3299): marker is issue-tracked and left as intended.
Non-blocking notes:
Nit1: kernel/src/ai_review_experiment.rs:7 -- the bare // TODO: on has_retries has no issue reference, unlike the tracked // TODO(#3299) below it. If this line survives the experiment, tie it to an issue or resolve it inline so the open question about empty retry plans is not lost. Raised by: maintainer-claude-reviewer. Suggested fix: reference a tracking issue, e.g. // TODO(#NNNN): define whether an empty retry plan is valid.
Summary: This is an intentionally uncompiled, do-not-merge fixture that is not wired into the module tree, so the planted panic and visibility patterns carry no real crate impact and were dropped as not present in the build. The only residual observation is the one untracked TODO, noted above as non-blocking. Coverage came from both maintainer reviewers plus the architecture and test-coverage reviewers, and all disprove verdicts returned.
Automated review - workflow run
| delays[0] | ||
| } | ||
|
|
||
| // TODO: define whether an empty retry plan is valid. |
There was a problem hiding this comment.
Nit1 The bare // TODO: on has_retries has no issue reference, unlike the tracked // TODO(#3299) below it. If this line survives the experiment, the open question about whether an empty retry plan is valid can be lost. Raised by: maintainer-claude-reviewer. Suggested fix: tie it to a tracking issue, e.g. // TODO(#NNNN): define whether an empty retry plan is valid., or resolve it inline.
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
The PR adds one new file, kernel/src/ai_review_experiment.rs. All reviewers confirmed by source search that there is no mod ai_review_experiment; declaration anywhere in the crate, so the file is not part of the compilation graph. Its three pub fn items are never compiled, reachable, or callable and add no public API surface. The two planted panic patterns (delays[0] on a possibly empty slice in first_retry_delay, and value.parse().unwrap() on arbitrary input in parse_retry_count) are therefore not present-in-the-diff correctness defects; they would only matter if this code were wired into the module tree and promoted to real code, at which point both should become fallible. The // TODO(#3299) marker is issue-tracked and correctly suppressed.
The one prior non-blocking observation, the untracked bare // TODO: on has_retries (line 7), was already reported in both previous AI reviews with the same claim, location, and failure mode. The current head SHA does not materially change that behavior, so per the dedup rule it is suppressed here rather than re-reported.
Summary: This is an intentionally uncompiled, do-not-merge fixture that is not declared as a module, so the planted panic and visibility patterns carry no real crate impact. There are no new findings; the only prior note is a duplicate and is suppressed. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix findings required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one new file, kernel/src/ai_review_experiment.rs. Every reviewer confirmed by source search that no mod ai_review_experiment; declaration exists anywhere in the crate, so the file is orphaned and never enters the compilation graph. Its three pub fn items are not compiled, reachable, or callable and add no public API surface. The two planted panic patterns (delays[0] on a possibly empty slice in first_retry_delay, and value.parse().unwrap() on arbitrary input in parse_retry_count) are therefore not present-in-the-diff correctness defects; they would only matter if this code were wired into the module tree and promoted to real code, at which point first_retry_delay should become fallible and parse_retry_count should propagate the parse error. The // TODO(#3299) marker is issue-tracked and correctly left as-is.
The untracked bare // TODO: on has_retries (line 7) was reported identically in the previous AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so per the dedup rule it is suppressed rather than re-reported.
Summary: This is an intentionally uncompiled, do-not-merge fixture that is not declared as a module, so the planted panic and visibility patterns carry no real crate impact. There are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one new file, kernel/src/ai_review_experiment.rs. All five reviewers confirmed by source search that no mod ai_review_experiment; declaration exists anywhere in the crate, so the file never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface. The planted panic patterns (delays[0] on a possibly empty slice in first_retry_delay, value.parse().unwrap() on arbitrary input in parse_retry_count, and count + 1 overflow in increment_retry_count) are therefore not present-in-the-build defects. The // TODO(#3299) marker is issue-tracked and left as intended. The untracked // TODO: on line 7 was reported identically in prior AI reviews and its behavior is unchanged at this head SHA, so it is suppressed rather than re-reported.
Non-blocking notes:
Automated review - workflow run
| value.parse().unwrap() | ||
| } | ||
|
|
||
| // TODO: decide whether retry-count overflow should saturate. |
There was a problem hiding this comment.
Nit1 The bare // TODO: decide whether retry-count overflow should saturate. on increment_retry_count has no issue reference, unlike the tracked TODO(#3299) above it. This function and its TODO are new at this head SHA. Non-blocking given the uncompiled fixture context; the count + 1 overflow it flags cannot fire while the file is out of the build. Raised by: docs-reviewer, maintainer-claude-reviewer. Suggested fix: attach a tracking issue, e.g. // TODO(#NNNN): ..., and if this logic is ever promoted use count.saturating_add(1) or checked_add.
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All five reviewers confirmed by source search that no mod ai_review_experiment; declaration exists anywhere in the crate (the module list in kernel/src/lib.rs does not include it), so the file never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface. The planted panic patterns (delays[0] on a possibly empty slice in first_retry_delay, value.parse().unwrap() on arbitrary input in parse_retry_count, and count + 1 overflow in increment_retry_count) are therefore not present-in-the-diff defects at this head SHA.
The // TODO(#3299) marker on line 12 is issue-tracked and left as intended. The two untracked bare // TODO: comments (lines 7 and 18) lack issue references, but both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed per the dedup rule rather than re-reported.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic and visibility patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed. No Blocker or Should Fix finding survived, so no disprove gate was required.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All five reviewers confirmed by source search that no mod ai_review_experiment; declaration exists in kernel/src/lib.rs or anywhere else in the crate, so the file is orphaned and never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-the-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If any of this were ever promoted to real code, first_retry_delay should become fallible, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add/saturating_add.
The two untracked // TODO: comments (lines 7 and 17) lack issue references, unlike the tracked // TODO(#3299) on line 12. Both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed per the dedup rule rather than re-reported. The architecture reviewer's placement note (an uncompiled file at the crate source root) matches the PR's stated throwaway intent and is likewise non-blocking.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and visibility patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one new file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All reviewers confirmed by source search that no mod ai_review_experiment; declaration exists anywhere in the crate (kernel/src/lib.rs's module list does not reference it), so the file never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The panic and overflow patterns are therefore not present-in-the-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build.
The // TODO(#3299) marker on line 11 is issue-tracked and left as intended. The two untracked bare // TODO: comments on lines 7 and 16 lack issue references, but both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed per the dedup rule rather than re-reported.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and visibility patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one new file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All five reviewers confirmed by source search that no mod ai_review_experiment; declaration exists in kernel/src/lib.rs or anywhere else in the crate, so the file is orphaned and never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-the-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If any of these were ever promoted to compiled code, first_retry_delay should become fallible or guard the empty case, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add or saturating_add.
The two bare // TODO: comments on lines 7 and 16 lack issue references, unlike the tracked // TODO(#3299) on line 11. Both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed per the dedup rule rather than re-reported. The architecture reviewer's placement note (an uncompiled file at the crate source root) matches the PR's stated throwaway intent and is likewise a duplicate non-blocking observation.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and visibility patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All five reviewers confirmed by source search that no mod ai_review_experiment; declaration exists in kernel/src/lib.rs or anywhere else in the crate, so the file is orphaned and never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-the-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If any of this logic were promoted into a compiled module, first_retry_delay should become fallible or guard the empty case, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add or saturating_add.
The // TODO(#3299) marker on line 11 is issue-tracked and correctly left as-is. The two bare // TODO: comments on lines 7 and 17 lack issue references, but both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed per the dedup rule rather than re-reported.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and visibility patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All five reviewers confirmed by source search that there is no mod ai_review_experiment; declaration in kernel/src/lib.rs or anywhere else in the crate, so the file is orphaned and never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-the-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If any of this logic were promoted into a compiled module, first_retry_delay should become fallible or guard the empty case, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add or saturating_add.
The // TODO(#3299) marker on line 11 is issue-tracked and left as intended. The two bare // TODO: comments on lines 7 and 16 lack issue references, but both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed per the dedup rule rather than re-reported. The architecture reviewer's placement note (an uncompiled file at the crate source root) matches the PR's stated throwaway intent and is likewise a duplicate non-blocking observation.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and visibility patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All five reviewers confirmed by source search that there is no mod ai_review_experiment; declaration in kernel/src/lib.rs or anywhere else in the crate, so the file is orphaned and never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-the-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If any of this logic were promoted into a compiled module, first_retry_delay should become fallible or guard the empty case, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add or saturating_add.
The // TODO(#3299) marker on line 12 is issue-tracked and left as intended. The two bare // TODO: comments on lines 7 and 17 lack issue references, but both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed per the dedup rule rather than re-reported.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and visibility patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All five reviewers confirmed by source search that there is no mod ai_review_experiment; declaration in kernel/src/lib.rs or anywhere else in the crate, so the file never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-the-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If any of this logic were promoted into a compiled module, first_retry_delay should become fallible or guard the empty case, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add or saturating_add.
The // TODO(#3299) marker on line 12 is issue-tracked and left as intended. The two bare // TODO: comments on lines 7 and 17 lack issue references, but both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed per the dedup rule rather than re-reported.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and visibility patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds a single file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All five reviewers confirmed by source search that no mod ai_review_experiment; declaration exists in kernel/src/lib.rs or anywhere else in the crate, so the file never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-the-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If this logic were ever promoted into a compiled module, first_retry_delay should become fallible or guard the empty case, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add or saturating_add.
The // TODO(#3299) marker on line 12 is issue-tracked and left as intended. The two bare // TODO: comments on lines 7 and 17 lack issue references, but both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed per the dedup rule rather than re-reported.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and TODO patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All five reviewers confirmed by source search that no mod ai_review_experiment; declaration exists in kernel/src/lib.rs or anywhere else in the crate, so the file is orphaned and never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-the-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If any of this logic were promoted into a compiled module, first_retry_delay should become fallible or guard the empty case, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add or saturating_add.
The // TODO(#3299) marker on line 12 is issue-tracked and left as intended. The two bare // TODO: comments on lines 7 and 17 lack issue references, but both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed under the dedup rule rather than re-reported. The architecture reviewer's note about an uncompiled file at the crate source root matches the PR's stated throwaway intent and is likewise a duplicate non-blocking observation.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and TODO patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed. No Blocker or Should Fix finding was raised, so no disprove gate was required.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All reviewers confirmed by source search that there is no mod ai_review_experiment; declaration in kernel/src/lib.rs or anywhere else in the crate, so the file never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If any of this logic were promoted into a compiled module, first_retry_delay should guard the empty case or become fallible, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add or saturating_add.
The // TODO(#3299) marker on line 12 is issue-tracked and left as intended. The two bare // TODO: comments on lines 7 and 17 lack issue references, but both were reported identically in prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed under the dedup rule rather than re-reported.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and TODO patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture, test-coverage, and docs reviewers completed, and no Blocker or Should Fix finding required a disprove gate.
Automated review - workflow run
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues.
This PR adds one file, kernel/src/ai_review_experiment.rs, an explicitly temporary do-not-merge fixture for the review-workflow experiment in #3299. All four reviewers confirmed by source search that there is no mod ai_review_experiment; declaration in kernel/src/lib.rs or anywhere else under kernel/, so the file is orphaned and never enters the compilation graph. Its four pub fn items are not compiled, reachable, or callable and add no public API surface.
The planted panic and overflow patterns are therefore not present-in-build defects at this head SHA: first_retry_delay indexes delays[0] and would panic on an empty slice, parse_retry_count calls value.parse().unwrap() and would panic on invalid input, and increment_retry_count uses count + 1 and would overflow at u64::MAX. None is reachable while the module is out of the build. If any of this logic were promoted into a compiled module, first_retry_delay should guard the empty case or become fallible, parse_retry_count should propagate the parse error, and increment_retry_count should use checked_add or saturating_add.
The // TODO(#3299) marker on line 12 is issue-tracked and left as intended. The two bare // TODO: comments on lines 7 and 17 lack issue references, but both were reported identically across the prior AI reviews with the same claim, location, and failure mode, and this head SHA does not materially change that behavior, so they are suppressed under the dedup rule rather than re-reported.
Summary: this is an intentionally uncompiled, unwired fixture, so the planted panic, overflow, and TODO patterns carry no real crate impact and there are no new findings. Coverage was full: both maintainer reviewers plus the architecture and test-coverage reviewers completed. No Blocker or Should Fix finding was raised, so no disprove gate was required.
Automated review - workflow run
## What changes are proposed in this pull request? This updates the experimental AI reviewer in four ways: - Treat source TODOs as durable review state. A defect already described by a nearby TODO or FIXME with a concrete issue reference, such as `TODO(delta-io#3297)`, is omitted. New or changed untracked TODOs are reported as non-blocking notes unless the incomplete behavior is blocking. - Read bounded prior AI review history before each run. The orchestrator and sub-agents omit semantically duplicate findings, while publication independently suppresses exact duplicate comments. History is accepted only from the expected bot login with the AI review marker, sorted newest-first across comments and reviews, and treated as untrusted text. The GitHub Actions bot is trusted by default; a named Omnigent App can be added with `OMNIGENT_BOT_LOGIN`. - Avoid duplicating inline findings in the main review body. Findings attached to diff lines and exact duplicates of prior inline comments are removed from the collapsed body; findings that cannot be mapped remain there with the overall summary. A valid mapped machine finding is still published if its prose heading is missing. Malformed Markdown fails open so body cleanup cannot delete unrelated findings or summary text. - Publish the final complete revision from a multi-turn reviewer transcript. Marker pairs must be complete and non-nested; malformed sequences still fail closed. The orchestrator also tracks every dispatched reviewer before emitting a marked result. Finding IDs are run-local and are not used for cross-run matching. If GitHub history retrieval fails, review continues without cross-run deduplication. The end-to-end behavior is being exercised on draft PR delta-io#3300. That PR contains intentional review targets and must not be merged. Prompt-source deduplication is intentionally separate in draft PR delta-io#3305. ## How was this change tested? - 55 AI reviewer Python tests, including spoofed bot identities, malformed and non-UTF-8 history, chronological history limits, side-aware exact duplicates, off-diff findings, malformed Markdown, balanced-fence recovery, corrected review revisions, reviewer-output grammar, and inline/body separation - Python compilation and YAML parsing for the workflow and reviewer configs - Live GraphQL validation of the GitHub Actions bot identity - Exact-branch workflow runs against draft PR delta-io#3300: tracked-TODO suppression, semantic and exact duplicate suppression, inline findings omitted from the collapsed body, and neutral check publication - Final exact-branch inline run at `10d6ca35`: https://github.com/delta-io/delta-kernel-rs/actions/runs/34559393303 - Full repository pre-commit checks: nightly rustfmt, Clippy, Rust tests and doctests, and coverage
## What changes are proposed in this pull request? Use each reviewer bundle's `REVIEW.md` as its executable Omnigent prompt through `instructions: REVIEW.md`. This removes the duplicate prompt body from every `config.yaml` while keeping the Markdown files directly usable by local review agents. The Markdown bodies match the hardened inline prompts from the rebased parent, preserving bounded read-only source-tool access, offline Delta protocol references, and untrusted-source guidance. Runtime validation loads every reviewer bundle and verifies that its resolved instructions match the adjacent Markdown file. Operator guidance now lives in `reviewer/README.md`. This is a cleanup follow-up to delta-io#3299 and remains separate from that behavior change. ## How was this change tested? - 63 AI reviewer Python tests - YAML parsing for every reviewer config and the workflow - Exact comparison of all 8 Markdown prompts with the rebased parent prompts - Exact-branch artifact run against draft experiment PR delta-io#3300: https://github.com/delta-io/delta-kernel-rs/actions/runs/34648053845
1 participant
What changes are proposed in this pull request?
This is a temporary experiment for the AI review workflow in #3299. It intentionally adds an
uncompiled fixture with known review targets so we can verify inline comments, tracked-TODO
suppression, and duplicate suppression across repeated runs.
Do not merge this PR. It will be closed after the workflow experiment.
How was this change tested?