Repository navigation
feat: implement issue #753 — [Phase 3] Default-off feature flag + regression guard + operator docs for the downstream-impact pass - #787
Conversation
…ression guard + operator docs for the downstream-impact pass
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 56 minutes and 26 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (20)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-19T08:44:19Z. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the downstream-impact pass feature, which maps changed files in a PR to impacted shared surfaces and downstream consumer repositories. It includes comprehensive documentation, a frozen golden manifest, several test cases, and a new BATS regression test suite (tests/test_downstream_impact_regression.bats) to pin the mapping logic. Feedback on the regression tests highlights a bug where the test suite fails on the empty changed-files case (07-empty) due to a missing changed.txt file, and suggests a more robust way to count directories in BATS without spawning subshells.
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: 8696017ba1661fbab3a819fae9054b008a8d9f16
Review mode: triage-approved (single reviewer)
Summary
Phase 3 wrap-up for the downstream-impact pass (epic #748, closes #753): adds a golden-fixture bats regression guard, frozen golden manifest + 7 expected-output cases, an operator doc (downstream-impact.md) linked from the documentation index, an AGENTS.md exception note, and registers the new bats file in lint.yml. Pure docs/tests/CI-registration — no runtime or scripts/review-one-pr.sh changes. The default-off flag behavior (AC#1) is exercised by the new off-path no-op tests; the gate itself already lives in scripts/lib/downstream-impact.sh from earlier epic stories.
Linked issue analysis
Closes #753. All four acceptance criteria are mapped to delivered files: (AC#2) regression guard pins the pure mapper against committed golden manifest + expected outputs and is registered in the always-run bats job; (AC#3) operator docs cover what the pass does, the DOWNSTREAM_IMPACT_ENABLED flag, GH_PAT cross-repo read scope, per-PR fetch/size caps, and manifest refresh; (AC#4) AGENTS.md updated with the new test exception and shellcheck is green; (AC#1) off-path no-op verified by two new flag-off tests asserting downstream_impact_triage_section emits nothing. Substantively addressed.
Findings
No blocking findings.
- gemini-code-assist (COMMENTED, non-blocking) claimed the 07-empty case fails due to a missing changed.txt. This is contradicted by the diff: 07-empty/changed.txt is committed as an empty blob (e69de29bb) and the bats CI job passed green. Concern is moot.
- gemini also suggested avoiding a subshell when counting case directories in the 'at least one golden case' test — a style nit, not a defect; test passes.
- MCP secret scan (run_secret_scanning) was unavailable in this environment (permission not granted); not run. The gitleaks CI secret scan passed and the diff is docs + JSON fixtures with no credential-like content, so no secret risk.
- Note: required CI is green; the only non-success checks (dev-lead dispatch/ci-relay) are CANCELLED self-superseded agent runs, not required gates.
CI status
All required checks green: bats, shellcheck, ShellCheck, Lint, validate-agent-profiles, gh-aw-compile, Compile agentic workflows, Agent Security Scan, Secret scan (gitleaks), CodeQL (actions + python), SonarCloud, unit-tests, AgentShield, holdout-guard, CodeRabbit status — all SUCCESS. CANCELLED entries (dev-lead / dispatch, dev-lead / ci-relay) are superseded Dev-Lead agent runs, not blocking gates. mergeStateStatus is BLOCKED only because org-leads review is still required (this verdict provides it).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Dev-Lead — review-changes (applied)Changes committed and pushed. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-19T17:19:53Z. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: 3c832446c9de27d3cc2150e9efb37d9b99dfa33e
Review mode: triage-approved (single reviewer)
Summary
Phase 3 wrap-up for the downstream-impact pass (epic #748, closes #753): adds a golden-fixture bats regression guard (tests/test_downstream_impact_regression.bats), a frozen golden manifest + 7 expected-output cases, an operator doc (downstream-impact.md) linked from the documentation index, an AGENTS.md test-list exception note, and registers the new bats file in lint.yml. Pure docs/tests/CI-registration — no runtime or review-one-pr.sh changes. +430/-2 across 20 files. Triage tier cleared this as low-risk; this confirmation review agrees.
Linked issue analysis
Closes #753 (Phase 3). Acceptance criteria map to delivered files: (AC#1) default-off cost cap — two new flag-off no-op tests assert downstream_impact_triage_section emits nothing when DOWNSTREAM_IMPACT_ENABLED is unset or false; (AC#2) regression guard pins the pure mapper (compute_downstream_impact) against a frozen, manifest-independent golden manifest + 7 expected outputs, with a catch-all test so new case dirs are auto-pinned, registered in the always-run bats job; (AC#3) operator docs cover what the pass does, the DOWNSTREAM_IMPACT_ENABLED flag, GH_PAT cross-repo read scope, per-PR fetch/size caps, and manifest refresh; (AC#4) AGENTS.md updated with the new test exception and shellcheck/Lint green. Substantively addressed.
Findings
No blocking findings.
- gemini-code-assist left 3 inline comments (2 medium, 1 low) on the bats file: (a) _assert_golden_case must guard a missing changed.txt for the 07-empty case, (b) drop the strict changed.txt requirement in the catch-all loop, (c) count case dirs with a pure-bash loop instead of ls|wc subshells. All three are resolved in the current HEAD (3c83244): _assert_golden_case now has an 'if [ -f changed.txt ]' guard defaulting to empty; the catch-all loop only requires expected.json; and the 'at least one golden case' test uses a pure-bash for-loop with count=$((count+1)). Author (dev-lead) replied to each thread documenting the fix.
- Prior bot approval existed on the earlier SHA 8696017 (now DISMISSED); the only changes since are a main merge and the gemini fix commit, both covered above.
- MCP secret scan (run_secret_scanning) was not run — the tool's permission was not granted in this environment. The gitleaks CI secret scan passed and the diff is docs + JSON fixtures + a bats test with no credential-like content (GH_PAT/GH_TOKEN appear only as variable names in docs), so no secret risk.
- Org-standards check: adding the new test file to the lint.yml bats list and documenting it in AGENTS.md follows the documented repo-specific exception — compliant.
CI status
All required checks green: bats, shellcheck, ShellCheck, Lint, validate-agent-profiles, gh-aw-compile, Compile agentic workflows, Agent Security Scan, holdout-guard, Secret scan (gitleaks), CodeQL (actions + python), SonarCloud (quality gate passed, 0 new issues), unit-tests, AgentShield, review, CodeRabbit — all SUCCESS. The only non-success entries (dependabot-automerge, several dependency-audit ecosystem jobs, dev-lead ci-relay) are SKIPPED no-op jobs, not blocking gates. mergeStateStatus is BLOCKED only pending the required org-leads review, which this verdict provides.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
Note I received your request but all AI engines are currently rate-limited. Please re-mention |
Dev-Lead — rate-limited (intent: on-mention)PR: #787 |
Superseded by automated re-review at
|
Dev-Lead — on-mention (no-changes)Engine ran but made no changes. |
|
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-19T20:21:05Z. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: fb1857c4641deee68a1f55af7d9fcd27bd3cd608
Review mode: triage-approved (single reviewer)
Summary
Phase 3 of the downstream-impact pass (issue #753, epic #748): adds a default-off feature flag's regression coverage and operator docs. Changes are a new golden-fixture bats regression guard (tests/test_downstream_impact_regression.bats), seven golden fixture cases + manifest, an operator guide (docs/pr-review-agent/downstream-impact.md) and doc-index entry, a one-line addition wiring the new test into the always-run bats job in .github/workflows/lint.yml, and an AGENTS.md exception note documenting that wiring. 430 additions / 2 deletions across 20 files, no production logic changes.
Linked issue analysis
Closes #753. The PR delivers exactly the Phase 3 scope: a regression guard pinning the pure mapper output against frozen golden fixtures, plus default-off operator documentation. The flag (DOWNSTREAM_IMPACT_ENABLED) and mapper logic land in earlier phases; this PR adds the guard + docs and includes off-path no-op tests asserting byte-identical behavior when the flag is unset/false. Substantively addressed.
Findings
No blocking findings. The lint.yml change is a single documented test-list addition that matches the pre-existing AGENTS.md exception for repo-specific bats files; not a security smell. Gemini (advisory bot) flagged earlier that _assert_golden_case might fail when changed.txt is absent for the empty case; the merged code already guards this with an 'if [ -f ... ]' fallback and the catch-all loop only requires expected.json, so the concern is resolved (confirmed by green bats/unit-tests checks). MCP secret scan was unavailable this run (permission not granted); the gitleaks CI check is green and scanned content is docs/test/fixture data referencing only env-var names (GH_PAT/GH_TOKEN), no secret material.
CI status
All required checks green: Lint, shellcheck/ShellCheck, bats, unit-tests, CodeQL (actions + python), Secret scan (gitleaks), SonarCloud (Quality Gate passed, 0 new issues), agent-shield, holdout-guard, validate-agent-profiles, gh-aw-compile, dependency-audit. CANCELLED/SKIPPED entries are redundant dev-lead dispatch/ci-relay runs and path-filtered audits, not failures. mergeStateStatus is BLOCKED pending this review.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Superseded by automated re-review at fb1857c.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Closes #753
Implemented by dev-lead agent. Please review.