7a765ce649
A local-first, agent-agnostic code-review skill for Asterinas, with a benchmark that measures its recall. It reviews code against the persona-keyed Coding Guidelines and writes one Markdown review file, making both objective (bug) and subjective (guideline-grounded) calls. Two review modes (scripts/resolve_target.sh), both anchored at HEAD: - `diff <base>` — the commit series merge-base(base,HEAD)..HEAD: each commit's message and diff, so the review covers per-commit intent and commit hygiene. - `files <path[:lines] ...>` — the working-tree contents of the named files. The raw argument string is self-tokenized (double quotes, line ranges) so the parse is deterministic regardless of how the skill is triggered. Execution model (SKILL.md, spec/execution_model.md): a thin orchestrator activates the five personas by path, fans out one isolated pass per persona (scripts/build_pass_prompt.sh assembles a cache-ordered prompt — stable contract and guideline first, the review input last), then deterministically assembles (scripts/assemble_review.sh), verifies, consolidates, and summarizes into the review file. `--per-persona-context=auto|yes|no` trades the fan-out (best recall) for a cheaper combined pass. Benchmark (benchmark/): one queryable problems.yaml, each problem pinned to a source commit and checked by validate_problem_yaml.sh; run.sh reconstructs each problem in an isolated worktree (fetching the commit if absent), reviews cheap-first with escalation to the fan-out on a miss, and grades recall — without leaking the answers to the reviewer (overlay_skill.sh overlays the current skill but excludes the suite, the worktree path is opaque, and the defects go only to the grader). Design rationale lives in spec/ (motivation, coding_guidelines, interface, execution_model, benchmark, related_work), with every significant decision argued in place. The deterministic scripts and the problem schema are covered by model-free tests in tests/. |
||
|---|---|---|
| .. | ||
| Makefile | ||
| README.md | ||
| lib.sh | ||
| test_assemble_review.sh | ||
| test_build_pass_prompt.sh | ||
| test_parse_pr_command.sh | ||
| test_post_reviews.sh | ||
| test_problems_schema.sh | ||
| test_resolve_target.sh | ||
README.md
Tests
Integration tests for aster-code-review's deterministic machinery
— the resolve_target.sh, build_pass_prompt.sh, and assemble_review.sh scripts,
plus a schema check of the benchmark's problems.yaml.
They are model-free, fast, and self-contained:
each test case builds a throwaway Git repository or fragment set,
exercises the script, and asserts on its output and exit status.
Run the whole suite from the skill root — it delegates here:
make test
For granular runs, this directory has its own Makefile:
make -C tests # all suites (the default goal)
make -C tests test_resolve_target # one suite
make -C tests help # list the discovered suites
Each command exits non-zero if any case fails.
Suites are auto-discovered (test_*.sh),
so adding one needs no Makefile edit.
Layout
One suite file per script, plus a shared library
— add a script, add a suite;
add an aspect, add a test_* function.
| File | What it holds |
|---|---|
Makefile |
Auto-discovers test_*.sh; default goal runs all suites, test_<suite> runs one. |
lib.sh |
Assert helpers (assert_eq, assert_contains, assert_absent, assert_before), the standard Git fixture builder (build_repo), and the case runner (run_suite). |
test_resolve_target.sh |
Cases for scripts/resolve_target.sh. |
test_build_pass_prompt.sh |
Cases for scripts/build_pass_prompt.sh. |
test_assemble_review.sh |
Cases for scripts/assemble_review.sh. |
test_problems_schema.sh |
Schema-validates benchmark/problems.yaml (via benchmark/validate_problem_yaml.sh). |
A suite sources lib.sh,
defines one test_<aspect> function per case (and an optional setup that prepares a fresh $TMP per case),
and ends with run_suite.
The runner discovers every test_* function,
runs each in its own scratch directory,
and reports ok / FAIL per case with a summary.
How this differs from benchmark/
The two are complementary and deliberately separate:
tests/ |
benchmark/ |
|
|---|---|---|
| Verifies | the machinery — change resolution and review assembly behave correctly | the review quality — does the skill find the planted defects? |
| Needs a model? | No (pure scripts) | Yes (persona passes + an LLM grader) |
| Speed | seconds | minutes |
| Headline | per-case ok/FAIL |
recall (target 100%) |
A bug in the deterministic primitives would corrupt every review and every benchmark run,
so these tests are the first line of defense;
benchmark/ then measures the judgement the scripts cannot.
What is covered
resolve_target.sh— the self-tokenizing argument grammar (missing mode, unknown mode/flag, lone positional, unbalanced quote each exit 2);diffmode (one base only, no range orbase..head, merge-base→working-tree resolution, the-dirtyhead marker); andfilesmode (quoted paths, merged/sorted line ranges, whole-file and range excerpts, missing-file error).build_pass_prompt.sh— arity/validation; the stable-then-volatile ordering (contract → persona + inlined guideline → review input); guideline inlining; and the cache property — the prefix up to the review input is byte-identical regardless of the input (and the input body never leaks into that prefix).assemble_review.sh— comment rendering, persona grouping and file→line ordering, dedup within a persona vs. keeping a comment across two personas, YAML title escaping, the--overwriterefuse-to-clobber guard, and arity errors.benchmark/problems.yaml—validate_problem_yaml.shschema check: unique ids, exactly one ofdiff/files, the named patch exists, ≥1 defect, per-defecttarget/persona/groundingfrom the known sets, andfixpresent iff notis_negative.