diff --git a/CHANGELOG.md b/CHANGELOG.md index 28072a7df7..c1f6a2f9d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up. ## [Unreleased] +- **Two extra hourly-review-repair contract assertions closed after `#1870`/`#1877` fixed the shared daily-cadence drift.** `#1870` already repaired `tests/test_pr_review_autofix_nvidia_nim_contract.py`'s core Clearfolio cron assertion (`test_review_fix_caller_keeps_the_github_daily_recovery_slot`, `cron: "23 7 * * *"`), and `#1877` independently repaired the remaining stale `_EXPECTED_TARGETS`/self-caller assertions in `tests/test_hourly_review_repair_callers.py` and `tests/test_github_hourly_conflict_repair.py`. This PR (`#1875`) adds the two assertions neither of those covered: `test_review_fix_caller_keeps_the_github_daily_recovery_slot` now also asserts the negative `cron: "23 */2 * * *"` never reappears, and asserts the reusable `pr-review-fix-scheduler.yml` engine never hard-codes `ContextualWisdomLab/clearfolio`, keeping the product-hourly-caller-neutral convention (`CLAUDE.md`) enforced by a real regression test rather than only code review. Also corrects `docs/product-technical-gap-baseline.md`'s matching entry, which still described the cron-staleness follow-up as "not yet fixed." - **Fix current-main contract drift that blocked the unscoped `agent-review-runtime-quality-ci.yml` "Verify scheduler and contextual-orchestrator review-repair contracts" step (which discovers and diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 21ee42e24c..73a5cb65f3 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -3203,10 +3203,31 @@ others) — this fix deliberately stayed scoped to the one file with direct, con starvation rather than a speculative sweep of every remaining occurrence. Worth revisiting each individually if queuing symptoms recur on them specifically. -**Separately found while validating this fix, not yet fixed:** `tests/test_pr_review_autofix_nvidia_nim_contract.py::test_review_fix_caller_runs_once_each_hour` -fails on a clean `origin/main` checkout, independent of this fix — `hourly-review-repair.yml` was renamed to -"Daily Review Recovery" and redesigned from one hourly cron to 17 staggered daily crons (one per target -repository), but this test still asserts the old single hourly `cron: "23 * * * *"`. Same bug class as the -`test_strix_quick_gate.sh` org-sweep-cron staleness found and fixed on `#1503` the same day: a test left -behind by a workflow redesign. Needs its own fix understanding the new staggered-daily design's actual -intended contract before rewriting the assertion — left for a dedicated follow-up rather than guessed at here. +**Update, now fixed on `main`:** the `tests/test_pr_review_autofix_nvidia_nim_contract.py::test_review_fix_caller_runs_once_each_hour` +staleness noted directly above (`hourly-review-repair.yml` renamed to "Daily Review Recovery" and redesigned +from one hourly cron to 17 staggered daily crons, one per target repository) was fixed by `#1870` itself — +renamed to `test_review_fix_caller_keeps_the_github_daily_recovery_slot` and repointed at Clearfolio's actual +`cron: "23 7 * * *"` — even though this note's own text lagged behind that fix. Investigating the new +staggered-daily design's full intended contract turned up two more files stale for the same root cause, +never updated for the same redesign: `tests/test_hourly_review_repair_callers.py` (`_EXPECTED_TARGETS`' 17 +schedule keys, and the schedule-list assertion, used the old hourly ` * * * *` shape instead of the +actual daily ` * * *` cron strings — 1 + 16 failing cases) and +`tests/test_github_hourly_conflict_repair.py::test_central_repository_has_hourly_self_caller` (asserted +`cron: "21 * * * *"` instead of the central `.github` self-caller's actual `cron: "21 6 * * *"`). Both were +fixed on `main` via `#1877` (the latter renamed to `test_central_repository_has_daily_self_caller`), +preserving each test's original protective intent (every distinct schedule maps to the right repository +exactly once; the central self-caller entry exists; the caller still dispatches to the reusable, +product-neutral `pr-review-fix-scheduler.yml` engine). Together with `#1870`'s fix, this closes the +confirmed live required-check failure on `ContextualWisdomLab/.github#1870`: `agent-review-runtime-quality-ci.yml`'s +"Verify scheduler and contextual-orchestrator review-repair contracts" step runs `pytest` with no positional +test path (full default `tests/` discovery), so it was picking up all of these stale assertions; with them +fixed, the exact unscoped `pytest` command the workflow runs passes cleanly (100% branch coverage across +`pr_review_conflict_scope`, `pr_review_autofix_context`, `zdr_policy`, and +`contextual_orchestrator_review_policy`). + +**Residual scope actually closed by `#1875`:** two regression assertions on the nvidia-nim contract test +that neither `#1870` nor `#1877` added — a negative check that the retired `cron: "23 */2 * * *"` cadence +never reappears, and a product-neutrality check that the reusable `pr-review-fix-scheduler.yml` engine never +hard-codes `ContextualWisdomLab/clearfolio` (enforcing this repo's thin-caller convention with a real +regression test rather than only code review) — plus this doc correction and the matching `CHANGELOG.md` +entry recording the investigation. diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 8d4397c42d..2073f1b25f 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -26,11 +26,30 @@ def _workflow_text(path: Path) -> str: def test_review_fix_caller_keeps_the_github_daily_recovery_slot() -> None: - """Keep the GitHub review repair caller on its distributed daily slot.""" + """Keep the GitHub review repair caller on its distributed daily slot. + + ``hourly-review-repair.yml`` (its `name:` is now "Daily Review Recovery") + was redesigned from a single cron shared by up to 18 thin per-repository + callers, firing every hour, into one file with 17 distinct daily cron + entries staggered across UTC hours -- see + docs/adr/0021-hourly-review-repair-single-file-consolidation.md. Each + resolved target, including this file's representative example + (Clearfolio, minute 23, hour 7), now runs once per day rather than once + per hour. The protected invariant this test originally guarded -- + dispatch reaches the reusable, product-neutral `pr-review-fix-scheduler.yml` + engine rather than reimplementing review-fix logic in the caller -- is + unchanged and reasserted here. + """ caller = _workflow_text(HOURLY_CALLER_WORKFLOW) assert 'cron: "23 7 * * *"' in caller assert 'cron: "23 * * * *"' not in caller + assert 'cron: "23 */2 * * *"' not in caller assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in caller + # Product-hourly-caller-neutral per CLAUDE.md/AGENTS.md: no target + # repository dispatched by the caller is hard-coded into the reusable + # engine itself. + reusable = _workflow_text(FIX_SCHEDULER_WORKFLOW) + assert "ContextualWisdomLab/clearfolio" not in reusable def test_scheduled_autofix_routes_through_contextual_orchestrator() -> None: