Skip to content

feat(evals): golden-dataset eval harness for the review skill - #4

Closed
Koan-Bot wants to merge 1749 commits into
mainfrom
koan.atoomic/review-skill-evals
Closed

Koan-Bot wants to merge 1749 commits into
mainfrom
koan.atoomic/review-skill-evals

Conversation

@Koan-Bot

@Koan-Bot Koan-Bot commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

Adds a skill-evaluation harness for the review skill — a deterministic
framework that scores the review skill's JSON output against a checked-in golden
dataset, so quality regressions are caught in CI and improvements are
measurable
across prompt iterations. Built generic so other LLM skills can
adopt the same discipline later.

Spec-kit pipeline (/speckit): specify → plan → tasks → implement, all
artifacts bundled below.

What lands

  • koan/app/skill_evals.py — data model + pure score_review() (reuses
    app.review_schema.validate_review as the single source of truth for
    validity; scores recall / LGTM-correctness / precision / blended score),
    run_eval(), load_cases(), a skill-keyed SCORERS registry, a live
    adapter that composes the existing review seams
    (build_review_prompt_run_claude_review_parse_review_json), and a
    CLI (python -m app.skill_evals review [--live] [--update-baseline]).
  • Golden dataset under koan/skills/core/review/evals/cases/:
    sql_injection, bare_except, hardcoded_secret (seeded bugs) and
    clean_refactor, benign_style (precision / false-positive traps), plus a
    baseline.json stub.
  • koan/tests/test_skill_evals.py — 45 offline tests + 1 opt-in
    @pytest.mark.slow live test.
  • Specs/docsspecs/skills/review.md, specs/components/skills.md,
    docs/operations/skill-evals.md, docs/README.md, README.md.

Two modes

Mode When LLM? Gate
Offline (default, CI fast group) every PR ❌ never
Live (regression/improvement tool) before/after a prompt change KOAN_EVAL_LIVE=1

The offline suite scores canned outputs and validates the dataset — it never
calls the Claude subprocess (per the project test rules). The live eval invokes
the real review pipeline, compares to baseline.json, and exits non-zero on
regression.

Test results (local)

  • make lint → ✅ all checks passed
  • koan/tests/test_skill_evals.py → ✅ 45 passed, 1 skipped (live, no env)
  • Adjacent regression checks → test_review_handler + test_review_schema
    (72) and test_skills (221) all green — the new evals/ data dir does not
    disturb skill discovery.
  • New-module coverage: 97.5% (repo baseline 88%, 0.5% tolerance — not at
    risk).

The full matrix (py 3.11 + 3.14, fast + slow-1/2/3 splits) runs in CI.

Spec-kit artifacts

specs/002-review-skill-evals/spec.md, research.md, plan.md,
tasks.md.

Review findings

Self-review found and fixed two issues in-branch (no unresolved findings):

  • fix(evals): run_eval left the report skill unbound on an empty case
    list → guarded with an empty-report short-circuit + regression test.
  • chore(evals): removed an unnecessary noqa (BLE001 isn't an enforced rule).

The live @pytest.mark.slow test is intentionally opt-in (KOAN_EVAL_LIVE)
and is the only path that touches the Claude subprocess — it skips in CI.

🤖 Generated with Claude Code

Koan-Bot and others added 30 commits June 25, 2026 00:43
Raise jira_outcome_publish.py coverage 76.5% -> 100%. Adds tests for
_fetch_pr_details (gh parse, error, empty, non-dict json), extract_pr_url
edge cases, _extract_failure_reason metadata-skipping and truncation,
upsert_jira_comment delegation, and the success-without-PR and
update-failure branches of publish_jira_mission_outcome.
Raise loop_manager.py coverage 81.5% -> 94.2% with behavioral tests:
- process_jira_notifications: disabled, config error, throttle, cold-start
  happy path, mention-skipped, empty-backoff, and exception paths
- in-process CLI handlers and main() dispatch (subprocess tests did not
  count toward coverage)
On KOAN_DEPLOY=railway, supervisord now starts the Flask dashboard on
0.0.0.0:5000 via a dedicated launcher that stays idle on non-railway
deploys. Because it binds to a public host, the dashboard is gated by a
single shared passphrase (KOAN_DASHBOARD_PWD): a before_request hook
redirects unauthenticated HTML to /login and returns 401 for /api/*,
and the launcher refuses to start without the passphrase. The session
secret is derived from the passphrase so logins survive re-deploys.
When the env var is unset (local use) the gate is inert.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When pre-mission git prep fails with an auth-looking error (403, "not
granted", permission denied, etc.), append an operator-facing diagnostic
block to the error: the env var that supplied the token (KOAN_GH_TOKEN
vs GH_TOKEN) with a short non-secret prefix, plus `gh auth status`
output. This surfaces the common Railway pitfall where the injected
GH_TOKEN is used instead of the KOAN_GH_TOKEN bot token. The full token
is never logged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Raise app/mission_executor.py coverage from 72.1% to 80.3% with a
dedicated test module. Covers _get_git_head outcomes, _maybe_retry_mission
skip/retry branches, the cli_skill translation paths of _handle_skill_dispatch
(combo expand, passthrough, translation, unknown/invalid-args), the matched
skill core-integrity recovery + stagnation-requeue path, and the
duplicate-insert branch of _maybe_escalate_to_debug.
Raise app/run.py coverage from 74% to 81% with behaviour-focused tests for
_notify_mission_normal branch selection, _notify_stagnation(_retry) message
shape + failure isolation, the _finalize_mission stagnation retry/cap state
machine, _run_preflight_check, and _handle_update_release.

Also replace a pre-existing try/except/pass in railway.py with
contextlib.suppress so make lint passes.
Replace try/except/pass around os.chmod with contextlib.suppress(OSError).
A red main on lint blocks CI for every branch.
Add 88 tests covering the previously-untested branches of the terminal
dashboard: module-level helper failures (_read, _load_config, _provider_*,
_coerce import/YAML errors), all four modal screens' button/action/submit
logic, keep-awake spawn lifecycle, status/web/pidfile helper exceptions,
config-tree build + drift rendering, usage-panel quota/no-quota branches,
and the focus/scroll/edit/toggle action guards.

Drive-by: replace a pre-existing try/except/pass with contextlib.suppress
in railway.py to clear a SIM105 lint failure already on main.
Adds DeepResearch._gather_file_hotspots() which ranks source files by
git churn over the last 200 commits. High-churn files are the most
likely to harbor tech debt and break under autonomous modification,
making them good DEEP-mode investment targets.

Hotspots surface in suggest_topics() as a new 'hotspot' source tier,
weighted below human priorities but above journal recaps. Test,
lockfile, and generated paths are excluded (reuses diff_triage sets).
Degrades to no suggestions when git is unavailable or the project has
fewer than 20 commits.

Closes Anantys-oss#2128
Adds behavioral coverage for get_notification_check_interval and
get_notification_max_check_interval: provider-override vs shared-section
precedence, floor clamping, provider isolation, and malformed/None/non-dict
value handling. The module backs github/jira poll cadence and had no tests.
…mory

The backtick/subshell shell-injection pattern listed bare interpreters
(bash/sh/python/ruby/perl) without word boundaries. Two failure modes:

1. Substring matches: `sh` matched inside "shadow", `nc` inside "Sync",
   so inline code spans in ordinary notes tripped the guard.
2. Bare interpreter mentions: legitimate notes like `python run.py` were
   flagged as injection.

Because sanitize_memory_entry() reuses scan_mission_text() on stored memory,
112 of the instance's own session summaries and learnings were being
replaced with "[BLOCKED: injection pattern detected]" on every read,
silently destroying accumulated context.

Narrow the pattern to genuinely dangerous tokens (curl/wget/nc/ncat/rm -rf)
with word boundaries. Dangerous interpreter use ("... | bash") is already
covered by the dedicated pipe-to-shell pattern, so true-positive coverage
is unchanged. Verified: all 6 attack samples still blocked, all 112 prior
false positives cleared.
feat(report): add weekly/monthly PR activity report
…ormal-mode gating

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…banners

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
atoomic and others added 27 commits June 30, 2026 18:45
Spec-kit design artifacts for native /speckit mission orchestration:
spec.md (FR-001..020, SC-001..010 + clarifications), plan.md, research.md,
data-model.md, contracts/skill-interface.md, quickstart.md, tasks.md, and
the requirements checklist. .specify/feature.json points speckit at this
feature dir; the SPECKIT agent-context marker in root CLAUDE.md links to
the plan.

Co-Authored-By: Claude <noreply@anthropic.com>
…ation helpers

Setup (Phase 1) + Foundational helpers for the spec-kit /speckit feature.

- koan/skills/core/speckit/ + speckit_from_branch/ SKILL.md (group: code,
  model_key: mission, github_enabled, github_context_aware) — placeholder
  scaffolds until handler + runner land.
- koan/app/speckit_orchestration.py — shared primitives: constitution gate
  (FR-003), repo:/branch: override parsing (FR-007), single-mission queuing
  via insert_pending_mission (FR-018), per-step outbox progress notes,
  project resolution. Load-bearing gates centralized (Principle VI).
- koan/app/config.py — get_speckit_config() accessor (quota_threshold 15,
  review_max_iterations 3, review_severity "important"); single read path.
- koan/tests/test_speckit_skill.py — 14 behavioral tests, all passing.
- plan.md/tasks.md — corrected the skill_dispatch integration model: a
  queued /speckit mission dispatches through a runner module +
  _COMMAND_BUILDERS entry like /implement (implement_runner +
  _build_implement_cmd), NOT "zero skill_dispatch changes".

Done: T001-T006, T008. Deferred to next Foundational increment (need
implement_runner + mission_executor dispatch reading): speckit_runner +
skill_dispatch registration (T008a-c) and the quota start-gate (T007).

Co-Authored-By: Claude <noreply@anthropic.com>
…, T008)

Co-Authored-By: Claude <noreply@anthropic.com>
…pt (US1)

US1 (MVP) + Foundational runner (T008a):

- speckit_runner.py — auto-discovered runner (rides
  _discover_runner_module + _build_generic_runner_cmd, so NO skill_dispatch
  registration needed — T008b is N/A). Constitution re-check (defense-in-depth,
  FR-003), builds the orchestration prompt, invokes Claude at mission tier
  via run_skill_loop/run_command_streaming.
- prompts/speckit.md — orchestration prompt: specify->plan->tasks->implement
  (commit per task), hard-abort on steps 1-4, best-effort review/CI
  (configurable iterations + severity), draft PR bundling artifacts.
- handler.py — bridge handler: resolves project, constitution gate, queues a
  single mission (FR-018). Handles chat + issue-URL first-arg.
- SKILL.md — wired handler: handler.py; bumped to 1.0.0.
- tests — 4 handler gate tests; 19/19 passing.

Remaining: T007 (quota start-gate in mission_executor), US2-US5, Polish.

Co-Authored-By: Claude <noreply@anthropic.com>
… prompt-injection hardening

Co-Authored-By: Claude <noreply@anthropic.com>
…lder contamination (T014b)

_substitute() did sequential str.replace per key, so a value containing
literal {OTHER_KEY} text (e.g. an operator goal, or untrusted issue text once
/speckit supports issue triggers) was re-scanned and could mangle or probe other
substitutions. Switched to a single regex pass: inserted values are not
re-scanned. Strict improvement for all prompts (Principle V prompt-injection
integrity). Regression tests added; 700 prompt-related tests still pass.

Co-Authored-By: Claude <noreply@anthropic.com>
…/T018 done

US3 (@mention trigger) is config-driven: the github_enabled +
github_context_aware flags set in US1's SKILL.md enable routing through
github_command_handler.validate_command. Added a test pinning that
/speckit and /speckit_from_branch are accepted by the @mention validator.
22/22 speckit tests pass.

Test-only commit (no production change) — ant-refactor/ant-review cycle
skipped: nothing to refactor or review.

Co-Authored-By: Claude <noreply@anthropic.com>
…warding (US2)

- handler.py: forward issue-URL + repo:/branch: tokens verbatim (previously
  stripped and discarded). The URL is the goal; the runner/agent fetches the
  issue content.
- speckit_runner.py: parse repo:/branch: from the goal (extract_overrides); a
  branch: token overrides the base branch; tokens are stripped from the prompt
  goal text (FR-007).
- tests: handler URL+token forwarding; runner branch-override + goal-stripping.
  24/24 passing.

Structured fetch_thread_context deferred — the prompt has Claude fetch the URL,
so the functional outcome (issue content as the problem) is achieved.

Co-Authored-By: Claude <noreply@anthropic.com>
…ls list

The CLAUDE.md split (main 0ccc9b4) moved the Core skills list to
koan/skills/CLAUDE.md; add the two new speckit skills alphabetically so
TestClaudemdSkillListSync passes and /help discovers them.

Co-Authored-By: Claude <noreply@anthropic.com>
`build_contemplative_command` resolved the binary against the project's
lightweight provider (`get_provider_for_role("lightweight", project_name)`),
but the model flags came from `get_contemplative_flags()` →
`get_claude_flags_for_role("contemplative")` with no `project_name`. For a
project with a per-project `cli.lightweight` override, the contemplative
binary was the project's provider while the model flag resolved against the
global provider — a newly-introduced mismatch flagged as important in the
PR review. The global (no per-project override) case was unaffected.

Forward `project_name` through `get_contemplative_flags` to
`get_claude_flags_for_role` and pass it from `run_contemplative_session`, so
the model matches its binary.

Co-Authored-By: Claude <noreply@anthropic.com>
… dedup regex

Two test_skills.py enforcement checks failed on CI because two new github_enabled
core skills were added without finishing the project's completeness gates:

- TestCoreSkillsComplete::test_all_core_skills_have_handlers —
  speckit_from_branch had no handler.py. Add a real from-branch handler
  (parse repo-id + branch, constitution gate, queue a single mission). This is
  US5's trigger handler (T024); the dedicated runner (specify-skip + branch-off
  git flow, T023/T025) remains the US5 follow-up.
- TestGithubActionRegexSync::test_all_github_enabled_commands_in_regex —
  speckit + speckit_from_branch were missing from _GITHUB_ACTION_RE in
  missions.py (mission dedup needs every github_enabled command there). Added.

Also: wire handler into speckit_from_branch/SKILL.md (v1.0.0), add 3 from-branch
handler tests. test_skills.py 221 passed; make lint clean.

Co-Authored-By: Claude <noreply@anthropic.com>
feat(provider): per-role CLI provider via cli: section
feat(skills): native /speckit — spec-kit mission orchestration
…e-fetch-limit

fix(deep_research): cap-limit open-PR coverage fetch (gh 30-default)
…ze-completed-debt

fix(deep_research): de-prioritize self-marked-done tech-debt topics
…fetch-paginate

fix(pr_review_learning): paginate review/comment fetches (Anantys-oss#2185)
…ics (Anantys-oss#2232) (Anantys-oss#2233)

* feat(memory): add MemoryMonitor RSS watchdog with tracemalloc diagnostics (Anantys-oss#2232)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(config): add memory_monitor config section and accessor (Anantys-oss#2232)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(run): memory watchdog samples RSS at loop top and self-restarts to reclaim (Anantys-oss#2232)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(memory): document tracemalloc diagnostic workflow for the watchdog (Anantys-oss#2232)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(dashboard): surface current RSS and watchdog state on /api/health (Anantys-oss#2232)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(memory): emit stderr diagnostics in memory_monitor broad except handlers (Anantys-oss#2232)

* fix(memory): report agent-loop RSS on /api/health and freeze watchdog knobs at startup (Anantys-oss#2232)

* fix: resolve CI failures on Anantys-oss#2233 (attempt 1)

* fix(memory): platform-correct RSS fallback, baseline restart-loop guard, and surfaced failure states (Anantys-oss#2232)

* fix(memory): disable watchdog on unreadable baseline, log /proc failures, isolate health endpoint (Anantys-oss#2232)

* fix(memory): honor 0.0-as-unknown RSS, report runtime watchdog state, schema-consistent health failures (Anantys-oss#2232)

* fix(memory): skip Linux-only /proc tests off-Linux, log self-RSS read failure (Anantys-oss#2232)

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Orphan-recovery PRs previously used a generic title and body ("fix:
recover orphan <branch>") regardless of the branch's actual work. Derive
the PR title and description programmatically from the branch's own
commits — no LLM call:

- title: subject line of the first commit
- single commit: body is that commit's full message
- multiple commits: body lists the first three commit messages

Falls back to the generic recovery message when no commits can be read,
so recovery still produces a usable PR.

Adds git_utils.get_commit_messages (oldest-first, %B companion to
get_commit_subjects), a new orphans skill spec, and a user-manual note.
…-pr-title-from-commits

feat(orphans): derive PR title/body from branch commits
Specify/plan/tasks for a deterministic skill-evaluation harness that scores
the review skill's JSON output against a golden dataset. Runs offline in CI
(scorer + dataset validity, no LLM) with an opt-in live mode (KOAN_EVAL_LIVE)
that invokes the real review pipeline and compares against a checked-in
baseline, so review-quality regressions are caught in CI and improvements are
measurable across prompt iterations. Generic scorer registry so other skills
can adopt the same discipline later.

Co-Authored-By: Claude <noreply@anthropic.com>
… stub

Adds koan/app/skill_evals.py — a deterministic skill-evaluation framework:

- Data model: EvalCase / CaseExpect / FindingExpect / CaseResult / EvalReport.
- score_review(): pure scorer reusing app.review_schema.validate_review as the
  single source of truth for validity; computes recall (file + keyword-stem +
  optional severity-band matching), lgtm correctness, precision penalty
(forbidden files flagged on clean code), a blended score, and strict pass.
- run_eval(): per-case scorer dispatch via a SCORERS registry; a review_fn that
  returns None or raises is recorded as errored and the run continues.
- load_cases(): discovers skills/core/<name>/evals/cases/*.json with strict
  validation and clear errors.
- Live adapter review_live_fn(): composes the existing review seams
  (build_review_prompt -> _run_claude_review -> _parse_review_json) with
  injectable deps for LLM-free unit testing; hermetic (no memory injection).
- compare_to_baseline()/write_baseline()/format_report(): per-metric
  improved/regressed/unchanged, non-zero exit on regression.
- CLI: 'python -m app.skill_evals review [--live] [--update-baseline]';
  --live is gated on KOAN_EVAL_LIVE so CI never invokes the Claude subprocess.

Golden dataset under skills/core/review/evals/cases/: sql_injection,
bare_except, hardcoded_secret (seeded bugs), clean_refactor and benign_style
(precision/false-positive traps), plus a baseline.json stub.

Covers spec tasks T001-T005, T008-T013.

Co-Authored-By: Claude <noreply@anthropic.com>
44 offline tests (fast CI group) covering score_review across all branches
(good/miss/wrong-severity/wrong-keyword/invalid/non-dict/none/clean-LGTM/
false-positive/unconstrained-lgtm/min-findings/file-mismatch), run_eval
aggregation + error handling, the scorer registry + fake-skill dispatch,
load_cases dataset validity + every malformed-case path, baseline
compare/write, review_live_fn with injected deps (no LLM), format_report,
and the CLI (offline summary, live-without-env refusal, live-with-env run,
--update-baseline).

Plus one @pytest.mark.slow live test that skips unless KOAN_EVAL_LIVE=1 —
the default suite never invokes the Claude subprocess.

Module coverage: 97.2%. Covers spec tasks T006-T007, T012, T014.

Co-Authored-By: Claude <noreply@anthropic.com>
- specs/components/skills.md: new "Skill evaluation harness" section + a
  checklist step to add eval cases for LLM-driven skills.
- specs/skills/review.md: an "Evaluation" section making the golden dataset,
  scored dimensions, offline/live modes, and the schema/prompt contract a
  documented part of the review skill.
- docs/operations/skill-evals.md: operator runbook — run offline checks, run the
  live eval, read the baseline/regression report, add a case, extend to another
  skill.
- docs/README.md: link the new doc.

Covers spec tasks T015-T018.

Co-Authored-By: Claude <noreply@anthropic.com>
… mention

- Live test reads KOAN_EVAL_LIVE through the public LIVE_ENV constant instead
  of reaching into the private _is_live_enabled() helper.
- README "Developer Experience" bullet points to the skill-evals runbook.

Co-Authored-By: Claude <noreply@anthropic.com>
run_eval left the report `skill` unbound when called with no cases, raising
NameError. Short-circuit to an empty report instead. Adds a regression test.
Found in self-review.

Co-Authored-By: Claude <noreply@anthropic.com>
BLE001 isn't an enforced ruff rule for this repo (PERF/SIM105/F541 only), so
the noqa was a no-op; keep the explanatory inline comment only.

Co-Authored-By: Claude <noreply@anthropic.com>
@Koan-Bot

Koan-Bot commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

Closing — opened against the wrong base. This fork's main is behind upstream; the canonical target is Anantys-oss/koan:main (where PRs from Koan-Bot branches merge, e.g. Anantys-oss#2259). Reopening there with the correct 17-file diff.

@Koan-Bot Koan-Bot closed this Jul 2, 2026
@Koan-Bot
Koan-Bot deleted the koan.atoomic/review-skill-evals branch July 2, 2026 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants