Repository navigation
[Phase 3] Default-off feature flag + regression guard + operator docs for the downstream-impact pass #753
Copy link
Copy link
Closed
Labels
dev-leadFor dev-lead agent pickupFor dev-lead agent pickupinitiativeEpic / initiative tracking issueEpic / initiative tracking issue
Description
Story
As a pr-review operator,
I want the downstream-impact pass shipped default-off behind a flag, covered by a golden-fixture regression guard, and documented,
so that the feature adds zero cost/behavior until I deliberately enable it, and its mapping logic cannot silently regress.
Acceptance Criteria
- The entire downstream-impact pass (assembly, inlining, verdict note) is gated behind a default-off env var (e.g.
DOWNSTREAM_IMPACT_ENABLED, defaultfalse); with the flag off,review-one-pr.shproduces byte-identical prompts/behavior to today (no fetches, no prompt changes). - A regression guard (bats) pins the pure mapper against a committed golden manifest + golden expected-output fixtures, so a change to mapping output fails CI; the new bats file is added to the
lint.ymlbats list as a stable check. - Operator docs under
docs/pr-review-agent/cover: what the pass does, how to enable it (the flag), theGH_PATscope it needs, the per-PR fetch + size caps, and how/when to refresh the consumer manifest (Story 1's builder). AGENTS.mdis updated if a new always-run workflow/test is introduced (per its documented exception convention);shellcheckpasses.
Tasks / Subtasks
- Add the
DOWNSTREAM_IMPACT_ENABLEDdefault-off gate around the pass inscripts/review-one-pr.sh; assert the off-path is a no-op. (AC: test issue from agent #1) - Add the golden-fixture regression bats (
tests/test_downstream_impact_regression.bats) with committed golden manifest + expected outputs; register it in.github/workflows/lint.yml. (AC: Go-live improvements for PR review agent #2) - Write operator docs (e.g.
docs/pr-review-agent/downstream-impact.md) and link fromdocs/pr-review-agent/documentation-index.md. (AC: Add @claude delegation, auto-merge, and rebase handling #3) - Update
AGENTS.mdif the regression guard becomes a documented repo-specific check; runshellcheck. (AC: Optimize review: small-PR and incremental fast paths #4)
Dev Notes
- Default-off is the cost cap: with
DOWNSTREAM_IMPACT_ENABLED=falsethere must be zero addedghcalls and zero prompt deltas — enforce with a test that diffs the off-path prompt against the pre-feature baseline. - Model the regression guard on the existing golden/holdout patterns:
tests/test_holdout_guard.bats(+scripts/lib/holdout-guard.sh) is the repo's reference for an immutable, CI-enforced guard;holdout-guard.ymlis a documented repo-specific exception in AGENTS.md. - Golden fixtures are the immutable baseline here — changing expected mapping output requires an explicit fixture update (visible in diff), preventing silent drift. This is fixture-based regression testing, not metric/eval tuning, so no held-out eval split is needed.
- Docs home is
docs/pr-review-agent/; add the page and link it fromdocumentation-index.md(existing index file). Note theGH_PATcross-repo read requirement (already satisfied by existing pr-review workflows per 💡 Cross-Repo Impact-Aware Review Annotations for Downstream Breakage Detection #653). - Any new bats file MUST be registered in the
lint.ymlbats list (AGENTS.md repo-specific exception); if a new always-run workflow is added, document it as an exception in AGENTS.md.
Project Structure Notes
Touches scripts/review-one-pr.sh (flag), tests/ (regression guard + fixtures), docs/pr-review-agent/ (operator docs), .github/workflows/lint.yml (bats registration), and possibly AGENTS.md.
References
- tests/test_holdout_guard.bats
- scripts/lib/holdout-guard.sh
- AGENTS.md#holdout-guard-exception
- docs/pr-review-agent/documentation-index.md
- .github/workflows/lint.yml#bats
Likely target surface
scripts/review-one-pr.shtests/docs/pr-review-agent/.github/workflows/lint.ymlAGENTS.md
Story prepared by the BMAD Scrum Master (Bob) for epic #748. Status: ready-for-dev.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
dev-leadFor dev-lead agent pickupFor dev-lead agent pickupinitiativeEpic / initiative tracking issueEpic / initiative tracking issue
Activity
don-petry commentedon Jun 19, 2026
Dev-Lead Implementation Plan
Issue: #753 — [Phase 3] Default-off feature flag + regression guard + operator docs for the downstream-impact pass
Scope
Ship the downstream-impact pass default-off behind
DOWNSTREAM_IMPACT_ENABLED(gate already wired in #752), add a golden-fixture regression guard pinning the pure mapper, and write operator docs.Implementation Checklist
DOWNSTREAM_IMPACT_ENABLEDdefault-off gate already wraps the whole pass (assembly + triage inline + verdict note) inscripts/review-one-pr.sh; no behavior when off.tests/fixtures/downstream-impact/golden/(frozen manifest + per-casechanged.txt/expected.json).tests/test_downstream_impact_regression.batspinningcompute_downstream_impactoutput against the golden fixtures + an off-path no-op assertion..github/workflows/lint.ymlbats list.docs/pr-review-agent/downstream-impact.mdand link it fromdocs/pr-review-agent/documentation-index.md.AGENTS.mdbats-list exception to document the regression guard; runshellcheck.Tests to Write
tests/test_downstream_impact_regression.bats— golden-fixture diff per case (direct workflow, indirect lib, indirect prompt, shared-source union, mixed, no-impact, empty) + flag-off no-op.Test Command
bats tests/test_downstream_impact_regression.bats(full suite viascripts/run-bats.sh); lint viabash .dev-lead/scripts/dev-lead-lint.shdon-petry commentedon Jun 19, 2026
Dev-Lead: Implementation Complete
Plan Execution
DOWNSTREAM_IMPACT_ENABLEDdefault-off gate already wraps the whole pass (assembly + triage inline + verdict note) — the gate was wired in [Phase 3] Wire the DOWNSTREAM_IMPACT block into the triage + deep prompts and surface it in the verdict #752; this story adds the regression guard, docs, and AGENTS.md record. (AC test issue from agent #1)tests/fixtures/downstream-impact/golden/(frozen manifest + 7 per-casechanged.txt/expected.json). (AC Go-live improvements for PR review agent #2)tests/test_downstream_impact_regression.batspinningcompute_downstream_impactagainst the golden fixtures, plus flag-off no-op assertions. (AC test issue from agent #1, Go-live improvements for PR review agent #2).github/workflows/lint.yml. (AC Go-live improvements for PR review agent #2)docs/pr-review-agent/downstream-impact.md(what the pass does, theDOWNSTREAM_IMPACT_ENABLEDflag,GH_PATscope, per-PR fetch + size caps, manifest refresh) and linked it fromdocumentation-index.md. (AC Add @claude delegation, auto-merge, and rebase handling #3)AGENTS.mdbats-list exception;shellcheck/dev-lead-lint pass. (AC Optimize review: small-PR and incremental fast paths #4)Test Results
Files Changed
tests/test_downstream_impact_regression.bats: new golden-fixture regression guard for the pure mapper + flag-off no-op checks.tests/fixtures/downstream-impact/golden/manifest.json: frozen golden manifest (independent of the liveconsumer-manifest.json).tests/fixtures/downstream-impact/golden/cases/*/{changed.txt,expected.json}: 7 golden cases (direct, indirect-lib, indirect-prompt, shared-source union, mixed, no-impact, empty)..github/workflows/lint.yml: registered the new bats file in the bats job list.docs/pr-review-agent/downstream-impact.md: new operator guide.docs/pr-review-agent/documentation-index.md: linked the new guide.AGENTS.md: documented the regression guard under the bats-list exception.Notes
DOWNSTREAM_IMPACT_ENABLED, default-off) was already implemented in [Phase 3] Wire the DOWNSTREAM_IMPACT block into the triage + deep prompts and surface it in the verdict #752; the off-path is a no-op (the assembly/fetch step inreview-one-pr.shis skipped when the flag is off, and the triage-section helper emits nothing), and this story adds the explicit flag-off no-op assertions to the new guard.batsjob, so no new workflow exception was added to AGENTS.md (only the bats-list exception was extended).consumer-manifest.jsonso Story 1's builder refreshing the manifest does not break the guard; only a mapper-output change does, requiring an explicit, reviewable fixture update.feat: implement issue #753 — [Phase 3] Default-off feature flag + reg…