docs(v2_reproducibility_harness): spec memo for #437 - #454
Merged
robotrocketscience merged 1 commit intoMay 6, 2026
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits 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 (1)
✨ 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 GuideAdds a detailed specification memo for the v2.0 reproducibility harness ( Sequence diagram for aelf bench all canonical runsequenceDiagram
actor User
participant AelfCLI
participant BenchSubcommand
participant BenchDispatcher
participant MabAdapter
participant LocomoAdapter
participant LongMemEvalAdapter
participant StructMemEvalAdapter
participant AmaBenchAdapter
participant CanonicalStore
User->>AelfCLI: invoke_aelf_bench_all_canonical
AelfCLI->>BenchSubcommand: route_bench_all
BenchSubcommand->>BenchDispatcher: run_all_adapters_with_headline_cut
BenchDispatcher->>MabAdapter: run_mab_headline
MabAdapter-->>BenchDispatcher: mab_metrics_with_tolerance
BenchDispatcher->>LocomoAdapter: run_locomo_headline
LocomoAdapter-->>BenchDispatcher: locomo_metrics_with_tolerance
BenchDispatcher->>LongMemEvalAdapter: run_longmemeval_headline
LongMemEvalAdapter-->>BenchDispatcher: longmemeval_metrics_with_tolerance
BenchDispatcher->>StructMemEvalAdapter: run_structmemeval_small_bench
StructMemEvalAdapter-->>BenchDispatcher: structmemeval_metrics_with_tolerance
BenchDispatcher->>AmaBenchAdapter: run_amabench_full
AmaBenchAdapter-->>BenchDispatcher: amabench_metrics_with_tolerance
BenchDispatcher->>BenchDispatcher: validate_headline_cut_matches_canonical
BenchDispatcher->>BenchDispatcher: assemble_schema_version_2_payload
BenchDispatcher->>CanonicalStore: write_v2_0_0_json
CanonicalStore-->>BenchDispatcher: write_ok
BenchDispatcher-->>User: exit_success
ER diagram for canonical results JSON schema_version_2erDiagram
CanonicalArtifact {
int schema_version
string label
string captured_at_utc
string git_commit
string aelfrice_version
string harness_version
}
HeadlineCut {
string id
}
AdapterHeadlineCut {
string adapter_name
string param_key
string param_value
}
AdapterResult {
string adapter_name
}
MetricGroup {
string group_name
}
Metric {
string metric_name
float value
}
ToleranceBand {
string band_kind
float band_pct
float lower
float upper
}
CanonicalArtifact ||--|| HeadlineCut : has_headline_cut
CanonicalArtifact ||--o{ AdapterResult : has_results
HeadlineCut ||--o{ AdapterHeadlineCut : defines_cut_for_adapter
AdapterResult ||--o{ MetricGroup : groups_metrics
MetricGroup ||--o{ Metric : contains_metric
Metric ||--|| ToleranceBand : has_tolerance_band
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 1 issue, and left some high level feedback:
- Consider tightening the description of the calibration-pass procedure for tolerance bands (e.g., explicit pseudo-steps or an example run sequence) so future maintainers can reproduce how
lower/upperwere derived without reading surrounding prose. - It might be helpful to specify in the canonical JSON schema how skipped adapters (e.g., missing
/tmp/...data) are encoded consistently, including expectedstatusvalues and how they interact with CI pass/fail, to avoid divergent interpretations during implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider tightening the description of the calibration-pass procedure for tolerance bands (e.g., explicit pseudo-steps or an example run sequence) so future maintainers can reproduce how `lower/upper` were derived without reading surrounding prose.
- It might be helpful to specify in the canonical JSON schema how skipped adapters (e.g., missing `/tmp/...` data) are encoded consistently, including expected `status` values and how they interact with CI pass/fail, to avoid divergent interpretations during implementation.
## Individual Comments
### Comment 1
<location path="docs/v2_reproducibility_harness.md" line_range="181" />
<code_context>
+
+- **Hyperparameter sweeps for the v2.0 numbers themselves.** This issue ships the harness, not the calibration. The first canonical run records whatever numbers v2.0 produces; tuning is separate.
+- **Cross-version regression tracking** (`v2.0.0` vs `v2.1.0` deltas). Out of scope until a v2.1 ship-gate exists.
+- **`big_bench` StructMemEval coverage.** ~10× runtime; spec'd as a follow-up issue once the small_bench numbers are stable.
+- **Public dashboard.** A web surface for the cron history is downstream of the harness existing at all.
+- **GPU-required benchmarks.** None of the current adapters need GPU; if a future adapter does, the policy lives in that adapter's add-issue, not this one.
</code_context>
<issue_to_address>
**issue (typo):** Possible typo or inconsistency between `--bench small` and `small_bench`.
Earlier we describe StructMemEval as running with `--bench small`, but here we mention "small_bench". If that’s not a distinct identifier, please align this wording with `--bench small` for consistency.
```suggestion
- **`big_bench` StructMemEval coverage.** ~10× runtime; spec'd as a follow-up issue once the `--bench small` numbers are stable.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| - **Hyperparameter sweeps for the v2.0 numbers themselves.** This issue ships the harness, not the calibration. The first canonical run records whatever numbers v2.0 produces; tuning is separate. | ||
| - **Cross-version regression tracking** (`v2.0.0` vs `v2.1.0` deltas). Out of scope until a v2.1 ship-gate exists. | ||
| - **`big_bench` StructMemEval coverage.** ~10× runtime; spec'd as a follow-up issue once the small_bench numbers are stable. |
There was a problem hiding this comment.
issue (typo): Possible typo or inconsistency between --bench small and small_bench.
Earlier we describe StructMemEval as running with --bench small, but here we mention "small_bench". If that’s not a distinct identifier, please align this wording with --bench small for consistency.
Suggested change
| - **`big_bench` StructMemEval coverage.** ~10× runtime; spec'd as a follow-up issue once the small_bench numbers are stable. | |
| - **`big_bench` StructMemEval coverage.** ~10× runtime; spec'd as a follow-up issue once the `--bench small` numbers are stable. |
|
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 |
Collaborator
Author
|
[claim:review:Toug:2026-05-06T20:58:04Z] |
Reproducibility harness — `aelf bench all` ship-gate for v2.0. Six contract calls teed up: CLI shape, "all" definition, canonical schema, tolerance bands, CI tiering, external-dep policy. Recommendation includes single-CLI dispatcher over existing adapter mains, sized headline cut, schema_v2 with per-metric tolerance bands, two-tier CI (nightly canonical + PR smoke). Adapters consumed unchanged: mab/locomo/longmemeval/structmemeval/amabench. Estimated implementation: ~250 LOC dispatcher + ~150 LOC tests + ~80 LOC CI + ~200 LOC docs.
robotrocketscience
force-pushed
the
docs/issue-437-reproducibility-harness-spec
branch
from
May 6, 2026 20:58
b1a9a36 to
76fe07c
Compare
Collaborator
Author
|
[release:review:Toug:2026-05-06T21:00:02Z] |
This was referenced May 6, 2026
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
2 participants
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.
Summary
Spec memo for #437 (v2.0 reproducibility harness —
aelf bench allship-gate). No code; six decisions teed up for ratification before implementation.What this memo decides
Six contract calls left open by the issue body:
aelf benchsubcommand vs freestandingpython -m benchmarks.run. Recommendation:aelf benchsubcommand insidecli.py.small_bench, MAB all four splits, etc.) vs full benchmark on every run.schema_version: 2forbenchmarks/results/v2.0.0.json, with per-metrictolerance_bandnext to each value./tmp/...dirs skip-with-status, LLM-judge runs offline-only.What ships if ratified
aelf benchCLI insrc/aelfrice/cli.py, dispatching to existing adaptermain()functions inbenchmarks/.benchmarks/run.pythin dispatcher.benchmarks/results/v2.0.0.jsoncanonical artifact (calibration-pass procedure documented)..github/workflows/bench-canonical.ymlnightly cron + ~30 LOC PR smoke inci.yml.tests/fixtures/bench_smoke/pinned offline fixtures (license attribution committed alongside).docs/COMMANDS.mdaelf benchentry.Estimated effort: ~250 LOC dispatcher + ~150 LOC tests + ~80 LOC CI + ~200 LOC docs. Adapter code unchanged.
Out of scope (deferred)
big_benchcoverage (~10× cost; follow-up).v2.0.0vsv2.1.0).Test plan
needs-specstate — implementation issue (or a slice issue) tracks the build.Summary by Sourcery
Documentation: