ci: trigger eval-calibration on the whole package (#1160)

[robotrocketscience]

Second acceptance criterion of #1160 — "Path filters include the modules each gate calibrates." PR #1194 did the ci.yml half; this is the eval-calibration.yml half. The umbrella stays open.

The defect

eval-calibration.yml pins aelf eval --json byte-for-byte against benchmarks/posterior_ranking/baseline.json. Its PR trigger filtered on three modules:

paths:
  - 'src/aelfrice/eval_harness.py'
  - 'src/aelfrice/calibration_metrics.py'
  - 'src/aelfrice/cli.py'

But the metric is produced by retrieve() — eval_harness.py:167 imports it, :182 calls it — so the whole retrieval and scoring stack sat outside the trigger. 25 modules are reachable from the harness; three were named.

The push: trigger carries no paths: key, and that asymmetry is what makes this a broken-main bug rather than merely thin coverage:

PR edits only scoring.py → job never runs on the PR → PR merges → the unconditional push-to-main run re-asserts the baseline → main goes red with no owning PR to revert.

The hidden coverage is real

Measured on main, reverting each mutation after:

mutation	roc_auc	spearman_rho
(baseline)	0.8443877551020408	0.5241037078990654
negate the bm25 term in scoring.py	0.7346938775510204	0.3571669713089927

The byte-exact assertion catches that the moment it is allowed to run.

Why src/aelfrice/** and not a longer list

#1160 offered either option. Enumerating is what drifted, and the coupling is not stable enough to enumerate safely — the harness pins l1_limit and passes entity_index_enabled=False, bfs_enabled=False, so I measured DEFAULT_L1_LIMIT (50 → 3) and DEFAULT_K1 (1.5 → 9.0) as byte-identical no-ops today. Which modules are live moves with the call. Over-triggering costs a 0.45–0.68 s measurement (timed, 3 runs); under-triggering costs a red main. ci.yml already filters on the package.

The guard derives its expectation

tests/test_eval_calibration_gate.py walks imports from the harness and asserts every reachable module matches the filter. It uses ast.walk, not tree.body, because the retrieve import sits inside a function body — a top-level-only scan misses exactly the dependency at issue, which is why the mutation test for that is included. Vacuity is pinned too: an empty walk would satisfy the coverage assertion for free.

Deliberately a new test module rather than an addition to tests/test_ci_path_filter.py, so this PR and #1194 can merge in either order without conflicting. Worth consolidating once both land.

Verification

• Full suite 6237 passed, 69 skipped (6234 baseline + 3 new).
• Mutation-verified, each in isolation: restoring the three-module list fails the coverage test; narrowing the walk to tree.body fails the vacuity test; adding paths: to the push trigger fails the third. No mutation left the suite green.
• Checked this does not strand my own open PRs. Widening the filter newly binds any PR touching src/aelfrice/**, so I ran aelf eval on the fix(retrieval): demote or exclude superseded beliefs, both arms behind a flag (#1187) #1191 (retrieval.py) and fix(lifecycle): dispose of aelfrice's own artifacts in ~/.aelfrice/ (#1186) #1190 branches: both byte-identical to the pinned baseline. No merge-order landmine.
• 3 signed commits; discretion grep clean on added lines.

Not fixed here — the same gate is still blind to its own name

benchmarks/posterior_ranking/ provides zero coverage of posterior ranking. Measured:

AELFRICE_POSTERIOR_WEIGHT=0.0 -> roc_auc 0.8443877551020408  spearman 0.5241037078990654
AELFRICE_POSTERIOR_WEIGHT=1.0 -> roc_auc 0.8443877551020408  spearman 0.5241037078990654
AELFRICE_POSTERIOR_WEIGHT=5.0 -> roc_auc 0.8443877551020408  spearman 0.5241037078990654

Byte-identical, and equal to the pinned baseline. eval_harness.py:129-131 builds every calibration belief at alpha=0.5, beta=0.5, so posterior_mean is constant and the posterior term is a constant offset that cannot reorder anything. Disabling the Bayesian rerank entirely would pass this gate.

That is a separate acceptance criterion — it needs varied per-belief posteriors in default.jsonl and therefore a deliberate baseline recut, which is a judgement call I have not made unilaterally. This PR makes the gate run on the right code; it does not make it sensitive to the posterior blend.

Summary by Sourcery

Broaden the eval-calibration CI gate to track changes across the whole aelfrice package and add tests that enforce the workflow’s triggers cover all modules contributing to the calibrated metric while keeping the push trigger unconditional.

CI:

• Update the eval-calibration workflow pull_request paths filter to include the entire aelfrice package instead of a short module list, plus related benchmark and config files.

Documentation:

• Document the eval-calibration trigger defect and its fix in the v4 changelog, including the rationale for widening the path filter.

Tests:

• Introduce tests that derive reachable modules from the eval harness, assert they are covered by the eval-calibration workflow’s PR paths filter, and verify the push trigger remains unfiltered.

Summary by CodeRabbit

• Bug Fixes

  • Corrected the evaluation calibration workflow so changes across the relevant scoring code reliably trigger the calibration gate.
  • Preserved unconditional checks for push events to prevent missed calibration failures.

• Documentation

  • Updated the v4.x changelog with details about the workflow fix and a remaining limitation involving posterior reranking calibration changes.

• Tests

  • Added coverage to verify workflow filters include all code affecting calibration metrics.

[sourcery-ai]

Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Review limit reached

@robotrocketscience, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

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, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5eab3437-3588-4876-b529-131c4e2bbca1

📥 Commits

Reviewing files that changed from the base of the PR and between d255507 and b0de048.

📒 Files selected for processing (3)

• .github/workflows/eval-calibration.yml
• CHANGELOG/v4.md
• tests/test_eval_calibration_gate.py

📝 Walkthrough

Walkthrough

The pull-request trigger for eval calibration now covers the full src/aelfrice/ tree. New tests derive the metric’s transitive import graph, verify path coverage, and ensure the push trigger remains unconditional. The v4 changelog documents these changes and a remaining posterior-weight limitation.

Changes

Eval calibration gate

Layer / File(s)	Summary
Broaden calibration trigger scope .github/workflows/eval-calibration.yml, CHANGELOG/v4.md	The workflow replaces enumerated source paths with src/aelfrice/**; the changelog records the corrected filter and remaining limitation.
Validate dependency and trigger coverage tests/test_eval_calibration_gate.py	Tests parse workflow paths, walk transitive aelfrice imports, verify every reachable module is covered, and enforce an unconditional push: trigger.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly reflects the main change: widening eval-calibration CI triggers to the whole package.
Description check	✅ Passed	The description covers the why, linked issue context, verification, test plan, and reviewer notes, though it is not fully template-formatted.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ci/issue-1160-calibration-filter

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sourcery-ai]

Reviewer's Guide

Expands the eval-calibration CI gate’s PR path filter from three specific modules to the entire aelfrice package, and adds tests that derive the dependency set from eval_harness imports to ensure the workflow’s triggers cover all code that affects the calibration metric while keeping the push trigger unconditional.

Flow diagram for widened eval-calibration CI path filter and guard test

flowchart TD
  PR["PR changes files"] --> F1{Files under src/aelfrice/**?}
  PR --> F2{Files under benchmarks/posterior_ranking/**?}
  PR --> F3{Changes to eval-calibration.yml or pyproject.toml?}

  F1 -->|yes| RUN["Run eval-calibration workflow on pull_request"]
  F2 -->|yes| RUN
  F3 -->|yes| RUN

  F1 -->|no| SKIP1["Do not run eval-calibration on PR"]
  F2 -->|no| SKIP2["Do not run eval-calibration on PR"]
  F3 -->|no| SKIP3["Do not run eval-calibration on PR"]

  RUN --> EVAL["Execute aelf eval --json"]
  EVAL --> CMP["Compare output to benchmarks/posterior_ranking/baseline.json"]

  subgraph Test_guard
    TSTART["tests/test_eval_calibration_gate.py"] --> TWALK["ast.walk over eval_harness imports"]
    TWALK --> TMODS["Derive reachable modules from retrieve stack"]
    TMODS --> TCHECK["Assert all reachable modules are matched by src/aelfrice/** filter"]
  end

  TCHECK --> TFAIL["Test fails if workflow paths drift from reachable modules"]

Loading

File-Level Changes

Change	Details	Files
Broaden the eval-calibration workflow’s pull_request path filter to cover the whole aelfrice package instead of enumerating specific modules, while keeping additional baseline-related paths and maintaining an unconditional push trigger.	Replace the three-module paths list under the pull_request trigger with a single src/aelfrice/** glob, justified via inline comments about retrieval/scoring dependencies and risk of broken main Retain existing filters for benchmarks/posterior_ranking, the workflow file itself, and pyproject.toml Ensure the push trigger in the workflow continues to have no paths filter so it still re-asserts the baseline on main pushes	.github/workflows/eval-calibration.yml
Document the eval-calibration gate defect and fix in the changelog, including the rationale for package-wide filtering and the remaining limitation around posterior rerank sensitivity.	Add a new bullet describing how the previous eval-calibration PR paths filter excluded retrieval/scoring modules, causing broken-main on push with no owning PR Explain that the workflow now filters on src/aelfrice/**, mirroring ci.yml, and summarize the performance vs. safety tradeoff Note that the gate still does not detect changes to posterior weighting due to a constant posterior in the current calibration corpus	CHANGELOG/v4.md
Introduce tests that derive modules reachable from eval_harness via AST import walking and assert that the workflow’s pull_request paths filter covers all of them, plus a guard that the push trigger remains unconditional.	Parse eval-calibration.yml to extract the pull_request paths list and enforce that a paths filter exists and is well-formed Implement GitHub-style glob matching to check whether each reachable aelfrice module’s path is covered by the PR paths filter Walk the eval_harness AST (using ast.walk to include function-body imports) to find transitively imported aelfrice.* modules and build the dependency set Add a vacuity guard test ensuring the import walk actually reaches key modules like eval_harness, retrieval, and scoring so the coverage assertion is meaningful Add a test that fails if any reachable module is not matched by the PR paths filter, recommending src/aelfrice/** over enumerating names Add a test that scans the workflow and fails if a paths filter is introduced under the push trigger, preserving the current hazard model	tests/test_eval_calibration_gate.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_eval_calibration_gate.py`:
- Around line 122-129: Update the path-matching helper used by
test_pr_filter_covers_every_module_the_metric_depends_on, specifically
_path_is_included, to process globs in declaration order: matching positive
patterns should include the path, while matching negated patterns should exclude
it, allowing later positive matches to re-include it. Replace any
order-insensitive any-based matching while preserving the existing default
behavior for paths with no matches.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c00f5dd-dc19-4c3c-b87b-af143091c23b

📥 Commits

Reviewing files that changed from the base of the PR and between fa169fc and d255507.

📒 Files selected for processing (3)

• .github/workflows/eval-calibration.yml
• CHANGELOG/v4.md
• tests/test_eval_calibration_gate.py

[robotrocketscience]

[claim:review:Setr:2026-07-30T16:53:56Z]

[robotrocketscience]

Approve. I re-ran both empirical claims rather than taking the tables on faith; both hold.

Hidden coverage — confirmed

Mutated scoring.py on main (flipping the sign inside relevance_pos = max(-bm25_raw, FLOOR), both sites — a different mutation than the one in the table, deliberately, to check the result isn't specific to one edit):

baseline  roc_auc 0.8443877551020408   spearman 0.5241037078990654
mutated   roc_auc 0.8852040816326531   spearman 0.5862197029093249

Byte-different from the pinned baseline. scoring.py was not in the old three-module filter, so that PR would have merged un-gated and the unconditional push run would have reddened main. The defect is exactly as described.

The posterior finding — confirmed, and it is the bigger problem

Reproduced on main:

AELFRICE_POSTERIOR_WEIGHT=0.0 -> roc_auc 0.8443877551020408  spearman 0.5241037078990654
AELFRICE_POSTERIOR_WEIGHT=1.0 -> roc_auc 0.8443877551020408  spearman 0.5241037078990654
AELFRICE_POSTERIOR_WEIGHT=5.0 -> roc_auc 0.8443877551020408  spearman 0.5241037078990654
pinned baseline                  roc_auc 0.8443877551020408  spearman 0.5241037078990654

Byte-identical across a 0→5 sweep, and identical to the pin. eval_harness.py:129-131 builds every calibration belief at alpha=0.5, beta=0.5, so posterior_mean is constant and the posterior term is a constant offset that cannot reorder anything.

Worth stating more bluntly than the PR does: a gate living at benchmarks/posterior_ranking/ would pass with the Bayesian rerank deleted. That is not thin coverage of posterior ranking — it is zero coverage, and the directory name asserts otherwise. Agreed it is a separate criterion and agreed it should not be resolved unilaterally, since it requires varied per-belief posteriors in default.jsonl and a deliberate baseline recut. But it should be filed with that framing rather than as a refinement.

Verified

• from aelfrice.retrieval import retrieve sits at eval_harness.py:167, inside a function body. Confirms ast.walk over tree.body is load-bearing, not stylistic.
• The walk resolves 25 modules, matching the claim.
• push: carries no paths: key; the asymmetry that turns a subset filter into a red main is real.
• Mutations caught, each in isolation: restoring the three-module list fails test_pr_filter_covers_every_module_the_metric_depends_on; narrowing the walk to tree.body fails test_the_import_walk_is_not_vacuous — and it is the vacuity test that catches it, which is the case for including it.

src/aelfrice/** over an enumeration is the right call, and the argument for it is the strongest thing in this PR: the measured no-op result for DEFAULT_L1_LIMIT and DEFAULT_K1 shows the live dependency set moves with the call, so any enumeration is a snapshot that starts drifting immediately. A sub-second over-trigger against a red main is not a close trade.

Notes, none blocking

1. The import walk silently drops dotted submodules. _reachable_modules resolves src/aelfrice/<name>.py only, so from aelfrice.wonder.lifecycle import ... yields "wonder.lifecycle", finds no file, and is dropped — along with everything it imports. I checked the live chain from eval_harness: nothing dotted is reachable today (the wonder.* imports are all under cli.py / mcp_server.py / hook.py, which the harness does not reach), so the walk is complete as of this commit. But the failure mode is silent under-reporting, and this guard exists precisely to catch drift. Resolving pkg/sub.py and pkg/sub/__init__.py is two lines. It doesn't matter while the filter is the whole package — it matters the moment someone narrows it, which is the case the guard is for.

2. test_the_push_trigger_stays_unconditional bounds the push block by splitting on the literal permissions:. If that key ever moves above on:, the bound disappears and the scan runs to EOF. It fails closed, so this is cosmetic.

3. Agreed on consolidating this with tests/test_ci_path_filter.py once ci: run pytest for benchmarks/ and scripts/ changes (#1160) #1194 lands. Keeping them separate to decouple merge order was the right call for now.

Merge mechanics

Needs a rebase — #1200 merged and moved main, and this touches [Unreleased]. I'll rebase and push once #1197 lands, so this only gets rewritten once. 3 signed commits, all other checks green.

[robotrocketscience]

Rebased onto ea5ca9ac (post-#1194, which merged the sibling ci.yml half). [Unreleased] collision resolved insert-only. Now 63e715c4: FF on main, 3 signed commits, same 167/3 diff stat, discretion grep clean on added lines, both path-filter guards green together.

Note for whoever picks up #1191: once this lands, any PR touching src/aelfrice/** is bound by the eval-calibration gate. #1191 touches retrieval.py and was already checked byte-identical against the pinned baseline, so no merge-order landmine there — but it now has to stay that way.

[robotrocketscience]

[release:review:Setr:2026-07-30T17:08:39Z]

[github-actions]

merge-train: blocked

1 review thread(s) are unresolved on these files: tests/test_eval_calibration_gate.py. Resolve them on the PR (click 'Resolve conversation' on each) and re-add the label.

The ready-to-merge label has been removed. Address the issue above and re-add the label when you're ready for another attempt.

[robotrocketscience]

CodeRabbit's negated-glob finding is valid, so I applied it in b0de0488 rather than dismissing it.

It is latent — the filter has no ! entry today — but the failure direction is the bad one. any(_glob_matches(...)) ignores negation, so src/aelfrice/** followed by !src/aelfrice/scoring.py would read as covered while the job actually skipped scoring.py. That is the guard failing open in precisely the case it exists to catch, which is worse than not having it.

Now matched in declaration order, mirroring how GitHub evaluates paths:: a later negative excludes, a later positive re-includes. Pinned by a unit test on the helper rather than by adding a negation to the real workflow — reverting the helper to any() fails that test, so it is load-bearing rather than decorative.

Still FF on ea5ca9ac, 4 signed commits, discretion clean.

[github-actions]

PR-size soft cap

This PR is over the advisory size threshold:

• 207 changed lines (limit: 200)
• 3 changed files (limit: 3)

Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated attn:merge-conflict cycles (see #602). When practical, split into smaller PRs that each touch a focused surface.

This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the size:override label and this comment will be removed on the next push.

[github-actions]

merge-train: merged b0de048 → main via FF push.