feat(eval): aelf eval subcommand + lift calibration harness to module (#365 R4) - #509
Merged
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Reviewer's GuideLifts the relevance-calibration harness into a reusable aelfrice.eval_harness module, updates audit_rebuild_log.py to delegate to it while preserving its CLI behavior, introduces an operator-facing Sequence diagram for the new aelf eval subcommand flowsequenceDiagram
actor Operator
participant AelfCLI as aelf_cli_main
participant EvalCmd as _cmd_eval
participant EvalHarness as eval_harness
participant Store as MemoryStore
participant Retrieval as retrieve
participant Metrics as calibration_metrics
Operator->>AelfCLI: run aelf eval [--corpus --k --seed --json]
AelfCLI->>EvalCmd: dispatch func with parsed args
EvalCmd->>EvalCmd: validate args.eval_k > 0
alt invalid k
EvalCmd-->>Operator: exit code 2 (usage error)
else valid k
EvalCmd->>EvalHarness: DEFAULT_CALIBRATION_CORPUS
EvalCmd->>EvalCmd: resolve corpus_path
EvalCmd->>EvalHarness: load_calibration_fixtures(corpus_path)
EvalHarness-->>EvalCmd: fixtures
EvalCmd->>EvalHarness: run_calibration_on_fixtures(fixtures, k, seed)
EvalHarness->>Store: build_calibration_store(fixture, seed)
Store-->>EvalHarness: MemoryStore instance
EvalHarness->>Retrieval: retrieve(store, query, l1_limit, flags)
Retrieval-->>EvalHarness: ranked beliefs
EvalHarness->>Metrics: precision_at_k / roc_auc / spearman_rho
Metrics-->>EvalHarness: metric values
EvalHarness->>Store: close()
EvalHarness-->>EvalCmd: CalibrationReport
alt args.eval_json is set
EvalCmd->>EvalCmd: build JSON payload dict
EvalCmd-->>Operator: JSON line (sorted keys), exit 0
else human-readable text
EvalCmd->>EvalHarness: format_calibration_report(report, corpus_path, seed)
EvalHarness-->>EvalCmd: text block
EvalCmd-->>Operator: text block, exit 0
end
end
Class diagram for the new eval_harness module and related typesclassDiagram
class EvalHarnessModule {
<<module>>
Path DEFAULT_CALIBRATION_CORPUS
int DEFAULT_K
int DEFAULT_SEED
+list~dict~ load_calibration_fixtures(path: Path)
+MemoryStore build_calibration_store(fixture: dict, seed: int)
+CalibrationReport run_calibration_on_fixtures(fixtures: Sequence~dict~, k: int, seed: int)
+str format_calibration_report(report: CalibrationReport, corpus_path: Path, seed: int)
}
class CalibrationReport {
+float p_at_k
+int k
+int n_queries
+int n_truncated_queries
+float roc_auc
+float spearman_rho
+int n_observations
}
class MemoryStore {
+MemoryStore(path: str)
+insert_belief(belief: Belief) void
+close() void
}
class Belief {
+str id
+str content
+str content_hash
+float alpha
+float beta
+str type
+str lock_level
+str locked_at
+int demotion_pressure
+str created_at
+str last_retrieved_at
}
class CalibrationMetricsModule {
<<module>>
+float precision_at_k(labels: list~bool~, k: int)
+float roc_auc(scores: list~float~, labels: list~bool~)
+float spearman_rho(scores: list~float~, labels: list~float~)
}
EvalHarnessModule --> CalibrationReport : returns
EvalHarnessModule --> MemoryStore : builds
EvalHarnessModule --> CalibrationMetricsModule : uses
MemoryStore --> Belief : stores
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
robotrocketscience
added
author-Gylf
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
cli._cmd_eval, the parser defaults and help text for--kand--seedare hard-coded (10 and 0); consider wiring these toeval_harness.DEFAULT_K/DEFAULT_SEEDso the CLI stays in sync with the harness defaults. - In
scripts/audit_rebuild_log.py,_load_calibration_fixturesis now just a thin alias aroundeval_harness.load_calibration_fixturesand appears unused within the module; consider removing it to avoid dead code and keep the delegation path clear.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `cli._cmd_eval`, the parser defaults and help text for `--k` and `--seed` are hard-coded (10 and 0); consider wiring these to `eval_harness.DEFAULT_K` / `DEFAULT_SEED` so the CLI stays in sync with the harness defaults.
- In `scripts/audit_rebuild_log.py`, `_load_calibration_fixtures` is now just a thin alias around `eval_harness.load_calibration_fixtures` and appears unused within the module; consider removing it to avoid dead code and keep the delegation path clear.
## Individual Comments
### Comment 1
<location path="scripts/audit_rebuild_log.py" line_range="218-223" />
<code_context>
- )
- from aelfrice.retrieval import retrieve # noqa: PLC0415
+def _run_calibration(corpus_path: Path, k: int, seed: int) -> int:
+ """Run the #365 R1 calibration harness; print report or error."""
+ eh = _load_aelfrice_eval_harness()
</code_context>
<issue_to_address>
**issue:** Guard against non-positive k before calling run_calibration_on_fixtures to avoid uncaught ValueError.
`eval_harness.run_calibration_on_fixtures` now raises `ValueError` for `k <= 0`, and this script passes `k` straight from the CLI without validation. That means `--k 0` (or negative) will produce a traceback instead of a controlled, non-zero exit. Adding a simple `if k <= 0: ...` check before calling into the harness (similar to `aelf eval`) would avoid exposing this internal exception and keep CLI behavior consistent.
</issue_to_address>
### Comment 2
<location path="src/aelfrice/eval_harness.py" line_range="51-56" />
<code_context>
print(f" rank {rank:>2}: {summary['packed_ranks'][rank]}")
-DEFAULT_CALIBRATION_CORPUS = (
- Path(__file__).resolve().parent.parent
- / "benchmarks"
- / "posterior_ranking"
- / "fixtures"
- / "default.jsonl"
-)
-DEFAULT_CALIBRATION_K = 10
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Resolve the default corpus path via package resources instead of repo-relative filesystem paths.
The current path computation (`Path(__file__).resolve().parent.parent.parent / "benchmarks" / ...`) assumes a specific source layout and may break once this is installed as a wheel, where that directory may not exist or be packaged. This can cause `DEFAULT_CALIBRATION_CORPUS.is_file()` to fail at runtime. To make this robust in installed environments, use `importlib.resources.files(...)` (or `open_text`/`open_binary`) to load the bundled JSONL from package data instead of walking up the filesystem tree.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+218
to
223
| def _run_calibration(corpus_path: Path, k: int, seed: int) -> int: | ||
| """Run the #365 R1 calibration harness; print report or error.""" | ||
| eh = _load_aelfrice_eval_harness() | ||
| if not corpus_path.is_file(): | ||
| print( | ||
| f"audit_rebuild_log: calibration corpus not found: " |
There was a problem hiding this comment.
issue: Guard against non-positive k before calling run_calibration_on_fixtures to avoid uncaught ValueError.
eval_harness.run_calibration_on_fixtures now raises ValueError for k <= 0, and this script passes k straight from the CLI without validation. That means --k 0 (or negative) will produce a traceback instead of a controlled, non-zero exit. Adding a simple if k <= 0: ... check before calling into the harness (similar to aelf eval) would avoid exposing this internal exception and keep CLI behavior consistent.
Comment on lines
+51
to
+56
| DEFAULT_CALIBRATION_CORPUS = ( | ||
| Path(__file__).resolve().parent.parent.parent | ||
| / "benchmarks" | ||
| / "posterior_ranking" | ||
| / "fixtures" | ||
| / "default.jsonl" |
There was a problem hiding this comment.
suggestion (bug_risk): Resolve the default corpus path via package resources instead of repo-relative filesystem paths.
The current path computation (Path(__file__).resolve().parent.parent.parent / "benchmarks" / ...) assumes a specific source layout and may break once this is installed as a wheel, where that directory may not exist or be packaged. This can cause DEFAULT_CALIBRATION_CORPUS.is_file() to fail at runtime. To make this robust in installed environments, use importlib.resources.files(...) (or open_text/open_binary) to load the bundled JSONL from package data instead of walking up the filesystem tree.
Owner
Author
|
[claim:review:kulili:2026-05-09T05:31:19Z] |
Owner
Author
|
Review pass — diff reads clean. Substantive checks:
Blocker — needs rebase before FF merge.
Please rebase onto Releasing my |
robotrocketscience
added
attn:unblock
Owner
Author
|
[release:review:kulili:2026-05-09T05:32:44Z] |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
Owner
Author
|
[claim:merge:setr:2026-05-09T05:43:48Z] |
Owner
Author
|
[claim:pr:Toug:2026-05-09T05:44:01Z] |
…rom script (#365 R4) Extract the calibration harness (load_calibration_fixtures / build_calibration_store / run_calibration_on_fixtures / format_calibration_report) from scripts/audit_rebuild_log.py into a wheel-installable module so the upcoming `aelf eval` subcommand and the existing script can share one implementation. Pure leaf module: imports only stdlib, aelfrice.calibration_metrics, and lazy-imports aelfrice.{models,store,retrieval} inside the run path. API change vs the script's inline shape: the runner is now run_calibration_on_fixtures(fixtures, k, seed) returning CalibrationReport, instead of run_calibration(corpus_path, k, seed) returning int + printing. Existence/empty checks move to call sites, so each surface emits its own prefix wording. Determinism contract preserved: same (fixtures, k, seed) -> identical report and identical formatted text. 12 unit tests cover: fixture loading skips malformed rows, seed controls noise shuffle, k validation, empty-fixtures guard, format output for defined and undefined metrics, truncated-line presence/ absence, and report determinism. Refs #365 (R4 Phase B preparation).
…val_harness (#365 R4) Replace the script's inline calibration helpers (_load_calibration_fixtures / _build_calibration_store / _run_calibration / _print_calibration_report / _format_optional_float / DEFAULT_CALIBRATION_*) with delegation to the new aelfrice.eval_harness module. Same exit codes, same stderr wording, same flag set. The script keeps a thin _load_calibration_fixtures alias so the existing unit test (tests/test_audit_rebuild_log.py:_load_calibration_fixtures import) continues to work without churn. Behavior gate: `python scripts/audit_rebuild_log.py --calibrate-corpus` emits the same byte-for-byte output before and after this change. All 16 audit_rebuild_log tests still pass.
… R4) Phase B of the close-the-loop calibration loop ratified at #317: operator-facing alias of the R1 audit_rebuild_log --calibrate-corpus mode, exposed as a top-level `aelf eval` subcommand. Flags: --corpus PATH override the bundled synthetic corpus --k N K for P@K (default 10) --seed N deterministic noise-shuffle seed (default 0) --json emit one sorted-keys JSON object instead of text Determinism contract preserved: same (corpus, seed, k) -> bytes-identical output across reruns, in both text and JSON modes. Sorted JSON keys give the future R5 CI status-check surface a diff-stable wire format. Exit codes match audit_rebuild_log: 0 report printed 1 corpus missing or empty 2 usage error (--k <= 0) 10 new CLI tests cover defaults, custom corpus, JSON shape, determinism in both modes, --k label propagation, --k validation, missing/empty corpus, and JSON-key sort stability. Closes Phase B of the R-rounds plan; R5 (CI synthetic-corpus aggregate) can now consume `aelf eval --json` directly. Refs #365.
Add src/aelfrice/slash_commands/eval.md and register `eval` in EXPECTED_COMMANDS so the slash-command parity test suite stays green after the previous commit added the `aelf eval` subcommand. Same template as locked.md / core.md / wonder.md — verbatim passthrough, allowed-tools restricted to Bash.
robotrocketscience
force-pushed
the
feat/issue-365-r4-aelf-eval
branch
from
May 9, 2026 05:44
2afe9b4 to
af4121a
Compare
robotrocketscience
added
attn:review
Owner
Author
|
Rebased onto |
Owner
Author
|
[release:merge:setr:2026-05-09T05:45:19Z] |
Owner
Author
|
[release:pr:Toug:2026-05-09T05:46:09Z] |
Owner
Author
|
[claim:review:Toug:2026-05-09T05:46:17Z] |
Owner
Author
|
[claim:review:setr:2026-05-09T05:46:44Z] |
Owner
Author
|
[release:review:setr:2026-05-09T05:46:49Z] |
Owner
Author
|
[release:review:Toug:2026-05-09T05:47:51Z] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase B of the close-the-loop relevance-calibration loop ratified at
#317 — exposes the R1 harness as an operator-facing
aelf evalsubcommand.
Shape
Three atomic commits + one slash-file follow-up:
feat(eval): aelfrice.eval_harness module— lift the calibrationharness (
load_calibration_fixtures/build_calibration_store/run_calibration_on_fixtures/format_calibration_report) out ofscripts/audit_rebuild_log.pyinto a wheel-installable leaf module.12 unit tests.
refactor(eval): audit_rebuild_log delegates calibration to aelfrice.eval_harness— the script keeps its CLI surface (flags,exit codes, stderr wording) and now imports the harness instead of
inlining it. Output is bytes-identical before and after; existing
16 audit_rebuild_log tests stay green.
feat(cli): aelf eval subcommand for relevance-calibration harness— adds the
evalsubparser with--corpus / --k / --seed / --jsonflags. 10 new CLI tests.
feat(slash): ship /aelf:eval slash file for the new subcommand—ships
slash_commands/eval.mdand registersevalin the slashparity test, so the visible CLI surface matches
EXPECTED_COMMANDS.Determinism contract
Same
(corpus, --k, --seed)→ bytes-identical output across reruns,in both text and JSON modes. JSON keys are sorted so the line stays
diff-stable across Python releases — relevant for the future R5 CI
status-check surface.
Verification
uv run aelf evalanduv run python scripts/audit_rebuild_log.py --calibrate-corpusproduce identical metric lines on the bundledsynthetic corpus (P@10=0.1000, ROC-AUC=0.8444, ρ=0.5241; n=7
queries, n_obs=35; seed=0).
Out of scope
R2 (hybrid labeling pipeline) and R3 (hand-label calibration) remain
lab-side per the issue spec. R5 (CI workflow on push to
main) isthe next public-repo round and can now consume
aelf eval --jsondirectly.
Refs #365.
Summary by Sourcery
Introduce a reusable relevance-calibration harness module and expose it via both the existing audit script and a new
aelf evalCLI subcommand.New Features:
aelfrice.eval_harnessmodule providing a reusable relevance-calibration harness API and defaults.aelf evalCLI subcommand to run the calibration harness against a synthetic corpus with configurable corpus, k, seed, and JSON output./aelf:evalslash command description for the new eval subcommand.Enhancements:
scripts/audit_rebuild_log.pyto delegate calibration mode to the sharedaelfrice.eval_harnessmodule while preserving its CLI surface and output.Tests:
aelfrice.eval_harnessmodule covering fixture loading, determinism, store construction, and report formatting.aelf evalcovering default behavior, JSON output, determinism, flag handling, and error cases.evalcommand.