Repository navigation
[Phase 2] Rewire the deep-review + rubber-duck tier prompts to consume pre-fed context #1104
Copy link
Copy link
Closed as not planned
Closed as not planned
Copy link
Labels
initiativeEpic / initiative tracking issueEpic / initiative tracking issue
Description
Story
As a pipeline maintainer,
I want update the deep-review and rubber-duck prompts to read the pre-fed metadata/diff files when present instead of running gh pr view / gh pr diff, while keeping dynamic exploration tools available,
so that the two tier-2 reviewers stop spending tool round-trips and reasoning tokens on deterministic re-fetching, capturing most of the ET saving.
Acceptance Criteria
- When the prefetch flag is on and
PR_CONTEXT_*files are present,prompts/deep-review.mdandprompts/rubber-duck.mdinstruct the model to read the pre-fed metadata + diff files instead of runninggh pr view/gh pr difffor the primary PR context. - When the flag is off (or the files are absent), both prompts fall back to the existing in-loop
gh pr view/gh pr diffsteps unchanged — behavior is byte-identical to today. - Both prompts explicitly retain permission to use Bash/Read/Grep/Glob and
gh apifor DYNAMIC needs (cross-file reads, org-standard lookups, secret-scan MCP) — only the deterministic metadata+diff fetch is replaced. - The pre-fed diff the deep tier consumes is the FULL diff (per Story 2), so the tier's reviewable surface is not reduced vs today.
- The deep-review held-out eval is run via
scripts/evals/run-eval.sh deep-review(5 cases, llm-judge scorer, pass_threshold 0.7) with the prompt change applied and the flag on, and its aggregate score is NOT lower than the flag-off baseline (no regression) — it is not required to improve.
Tasks / Subtasks
- Add a conditional 'pre-fed context' inputs section to deep-review.md and rubber-duck.md (AC: test issue from agent #1, Go-live improvements for PR review agent #2)
- Reference PR_CONTEXT_METADATA_FILE / PR_CONTEXT_DIFF_FILE the same way $SAFETY_CHECKS_FILE is referenced
- Keep the gh pr view/diff steps as the documented fallback when the files are absent
- Preserve dynamic-tool guidance (AC: Add @claude delegation, auto-merge, and rebase handling #3)
- Keep the FORBIDDEN-enumeration guardrail and the gh api standards path intact
- Confirm full-diff consumption (AC: Optimize review: small-PR and incremental fast paths #4)
- Run the deep-review held-out eval and confirm no regression (AC: feat: add Copilot engine support via REVIEW_ENGINE toggle #5)
- Use scripts/evals/run-eval.sh deep-review against evals/deep-review/holdout
- Do NOT modify held-out cases (holdout-guard.sh enforces immutability)
Dev Notes
- The deep tier reads
$SAFETY_CHECKS_FILE,$ADVISORY_BOT_FEEDBACK_FILE,$DOWNSTREAM_IMPACT_FILEtoday (prompts/deep-review.mdInputs). Add$PR_CONTEXT_METADATA_FILE/$PR_CONTEXT_DIFF_FILEthe same way, with an explicit 'if set, read these INSTEAD of gh pr view/diff; if unset, run the gh commands as before' instruction — this keeps the flag-off path byte-identical (required for rollback + holdout-eval stability, per the SAFETY_CHECKS precedent). - Do NOT weaken the tier's scope guardrails: keep the
FORBIDDEN: gh search prs / gh pr list / enumerationblock and the secret-scan MCP step. The model still needs Bash/Read/Grep/Glob (granted byengine.sh run_agentic/run_duck) for dynamic cross-file analysis — only the two deterministic fetches move to files. - rubber-duck runs cross-engine (often a different model family, e.g. Gemini via
run_duck); its prompt'sgh pr view --json ...,repository,...field list differs slightly from deep's — ensure the pre-fed metadata superset (Story 2) coversrepositorytoo, or the duck prompt reads defensively. - Regression gate:
scripts/evals/run-eval.sh deep-reviewscores againstevals/deep-review/holdout/cases.jsonlusing the llm-judge scorer (pass_threshold in scorer.json). The held-out cases are immutable (scripts/lib/holdout-guard.sh, CODEOWNERS over evals/) — never edit them to make the number move. - This story is independently reviewable: it touches only the two prompt files and their eval run; it depends on Story 2's plumbing but not on Story 4.
- Eval mechanics:
scripts/evals/run-eval.shexecutes the skill prompt once per held-out case viaengine.sh run_triagewith the case context INLINED (not fetched via tools). So the eval already exercises pre-fed-style context — a valid guard that the rewritten prompt still reasons correctly on inlined metadata/diff. It cannot catch a full-diff-vs-truncated mismatch; that is covered by Story 2's full-diff constraint and Story 6's A/B smoke.
Project Structure Notes
Touches prompts/deep-review.md and prompts/rubber-duck.md only. Eval invocation via existing scripts/evals/run-eval.sh; no eval-case edits (guarded).
References
- prompts/deep-review.md#inputs
- prompts/rubber-duck.md#steps
- scripts/evals/run-eval.sh
- evals/deep-review/holdout/cases.jsonl
- scripts/lib/holdout-guard.sh
Likely target surface
prompts/deep-review.mdprompts/rubber-duck.md
Story prepared by the BMAD Scrum Master (Bob) for epic #1101. Status: ready-for-dev.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
initiativeEpic / initiative tracking issueEpic / initiative tracking issue
Activity
don-petry commentedon Jul 10, 2026
Dev-Lead Implementation Plan
Issue: #1104 — [Phase 2] Rewire the deep-review + rubber-duck tier prompts to consume pre-fed context
Scope
Update
prompts/deep-review.mdandprompts/rubber-duck.mdso that when the Story 2 prefetch flag is on and thePR_CONTEXT_METADATA_FILE/PR_CONTEXT_DIFF_FILEfiles are present, the model reads them instead of runninggh pr view/gh pr diff; when unset/absent, fall back to the existing gh commands unchanged.Implementation Checklist
prompts/deep-review.md: add$PR_CONTEXT_METADATA_FILE/$PR_CONTEXT_DIFF_FILEto Inputs (same style as$SAFETY_CHECKS_FILE); rewire steps 3 (gh pr view) and 4 (gh pr diff) to read the pre-fed files when set, gh fallback when unset (AC test issue from agent #1, Go-live improvements for PR review agent #2, Optimize review: small-PR and incremental fast paths #4)prompts/deep-review.md: explicitly retain Bash/Read/Grep/Glob +gh apifor dynamic needs; keep FORBIDDEN block + secret-scan MCP step intact (AC Add @claude delegation, auto-merge, and rebase handling #3)prompts/rubber-duck.md: same Inputs + steps 2/3 rewire; note metadata superset coversrepository; retain dynamic-tool + FORBIDDEN guardrails (AC test issue from agent #1–Optimize review: small-PR and incremental fast paths #4)Tests to Write
tests/test_prefetch_context.batscovers Story 2 plumbing; no prompt-content assertion tests exist to extend.Test Command
bash .dev-lead/scripts/dev-lead-lint.sh+scripts/evals/run-eval.sh deep-review(held-out regression gate, AC #5)don-petry commentedon Jul 10, 2026
Dev-Lead: cannot implement issue #1104 — needs human attention
A stage timed out (exit 124) while implementing this issue. A timeout is treated as a first-class, non-retryable condition — retrying with the same budget would just time out again — so this escalates for human review on the first occurrence instead of wasting a same-budget retry.
timeoutThis hit the max action budget. Split this issue into smaller, independently-implementable pieces, or raise
vars.ACTION_TIMEOUT_SEC, then re-apply thedev-leadlabel.don-petry commentedon Jul 14, 2026
Split & superseded. dev-lead timed out (exit 124, 1800s action budget) implementing both prompt files in one pass — a non-retryable timeout. Per the escalation guidance, this is split into two independently-implementable, per-file stories that each fit the action budget:
prompts/deep-review.md(carries the deep-review held-out eval regression gate)prompts/rubber-duck.mdBoth are labeled
dev-leadand linked as sub-issues of epic #1101. Closing this as superseded; no code from the timed-out run was recoverable (ephemeral runner, unpushed branch). — Claude Code