| 1 | --- |
| 2 | title: Search-quality eval is manual by default, not a CI gate on every PR |
| 3 | date: 2026-05-10 |
| 4 | category: docs/solutions/architecture |
| 5 | module: skills/last30days/scripts/evaluate_search_quality.py |
| 6 | problem_type: design_decision |
| 7 | component: ci_policy |
| 8 | severity: low |
| 9 | applies_when: |
| 10 | - a contributor proposes wiring search-quality eval into PR CI |
| 11 | - a change affects retrieval, ranking, grounding, or synthesis quality and a reviewer asks "why aren't we testing this in CI?" |
| 12 | - someone is deciding whether a new evaluator-style script belongs in the default CI workflow |
| 13 | related_components: |
| 14 | - search_quality_evaluation |
| 15 | - ci_workflow |
| 16 | - llm_judging |
| 17 | tags: |
| 18 | - ci-policy |
| 19 | - eval |
| 20 | - design-decision |
| 21 | - cost-vs-signal |
| 22 | - non-determinism |
| 23 | - manual-gates |
| 24 | --- |
| 25 | |
| 26 | # Search-quality eval is manual by default, not a CI gate on every PR |
| 27 | |
| 28 | ## Context |
| 29 | |
| 30 | `skills/last30days/scripts/evaluate_search_quality.py` compares a baseline revision against a candidate revision across a fixed pool of reviewer topics. It produces two flavors of metrics: deterministic overlap (Jaccard, retention) and LLM-judged quality scores. The natural impulse on seeing an evaluator script is to wire it into CI on every PR — "regression catcher, run it automatically." We deliberately don't. |
| 31 | |
| 32 | Three properties of this particular evaluator make CI-on-every-PR the wrong default: |
| 33 | |
| 34 | 1. **Live API access.** The candidate revision typically needs the engine to actually run, which means real ScrapeCreators calls, real reddit fetches, real YouTube searches. CI runs would either need production credentials or a record/replay fixture set that drifts almost immediately as external APIs change shape. |
| 35 | |
| 36 | 2. **Cost and latency.** A full eval pass runs the pipeline N times across reviewer topics. Multiplied by every PR (including doc-only PRs), the spend is meaningful and the wall-clock pushes CI from ~30s to many minutes. |
| 37 | |
| 38 | 3. **Non-determinism in the judging path.** The LLM-judged metrics are valuable for review but depend on judge-model behavior on a given day. A flaky eval that fails 1 PR in 20 because the judge re-scored an item differently is a worse CI signal than no eval at all — it teaches contributors to retry rather than read the result. |
| 39 | |
| 40 | The deterministic overlap metrics are useful regression signals but they are not the same as user-facing correctness. A change that improves overlap can degrade synthesis quality; a change that drops overlap can be a deliberate improvement. So even the deterministic side isn't safe to auto-fail on. |
| 41 | |
| 42 | ## Guidance |
| 43 | |
| 44 | ### 1. Keep search-quality eval available, just not automatic |
| 45 | |
| 46 | The script stays runnable by maintainers and contributors. The pattern is: |
| 47 | |
| 48 | ```bash |
| 49 | LAST30DAYS_PYTHON=python3.13 \ |
| 50 | python3 skills/last30days/scripts/evaluate_search_quality.py \ |
| 51 | --baseline main --candidate HEAD |
| 52 | ``` |
| 53 | |
| 54 | Reviewers can request a manual eval run when a PR is in the retrieval/ranking/synthesis path and the risk warrants it. Contributors can run it locally before submitting if they want signal upfront. |
| 55 | |
| 56 | ### 2. Standard PR CI gates remain deterministic and contract-shaped |
| 57 | |
| 58 | `pytest` (offline-safe), plugin-contract checks, version-consistency contracts, ruff/lint. Anything that returns the same answer twice for the same input. Quality-of-output assessment lives outside that loop. |
| 59 | |
| 60 | ### 3. The middle ground is `workflow_dispatch`, not auto-PR-gating |
| 61 | |
| 62 | If maintainers want a GitHub-triggered eval that doesn't make every PR pay the live-API cost, the right shape is a manually-dispatched workflow (or a label-triggered one) — not a `pull_request:` workflow that runs unconditionally. That keeps the cost knob in human hands. |
| 63 | |
| 64 | ### 4. Revisit if the eval can ever be made offline-deterministic |
| 65 | |
| 66 | The blocker is the live-API + non-determinism combination. If a future iteration of the script can compute meaningful Jaccard/retention metrics against static fixtures (no live API calls, no LLM judging), the decision flips and it becomes a candidate for default CI. The decision below tracks that condition; revisit when it's met. |
| 67 | |
| 68 | ## What this means in practice |
| 69 | |
| 70 | - Don't merge PRs that wire `evaluate_search_quality.py` into the default `validate.yml` workflow. |
| 71 | - Do merge PRs that add `workflow_dispatch` triggers or label-gated runs. |
| 72 | - When reviewing a retrieval/ranking change, request a manual eval if the diff suggests it could regress quality — don't expect CI to catch it. |
| 73 | |
| 74 | ## Links |
| 75 | |
| 76 | - `skills/last30days/scripts/evaluate_search_quality.py` — the evaluator script |
| 77 | - `docs/search-quality-eval.md` — user-facing usage documentation |
| 78 | - `.github/workflows/validate.yml` — the default CI workflow (deterministic gates only) |
| 79 | |
| 80 | --- |
| 81 | |
| 82 | *Adapted from a draft ADR proposed by @hnshah in [#374](https://github.com/mvanhorn/last30days-skill/pull/374), restructured into the `docs/solutions/` convention. The original ADR text correctly identified the constraint; this version adds the "why workflow_dispatch is the middle ground" framing and the revisit-condition.* |
| 83 |