Skip to content

feat(algorithm): on-demand reasoning skills + an advisory proposal-quality declaration (#140) - #246

Open
OsherElhadad wants to merge 13 commits into
mainfrom
feat/issue-140-reasoning-skills
Open

feat(algorithm): on-demand reasoning skills + an advisory proposal-quality declaration (#140)#246
OsherElhadad wants to merge 13 commits into
mainfrom
feat/issue-140-reasoning-skills

Conversation

@OsherElhadad

Copy link
Copy Markdown
Collaborator

Closes #140

Builds on #199 (the algorithm hub) and stacks on #222 (feat/issue-129-failure-memory, whose base already carries #199 + #212). The reviewer's "✅ one copytree" assessment held: the file half of this is literally one copytree on #199's inject seam.

What the skills are and when they load

A new reasoning skill component: tiny skills loaded at ONE step to counter ONE named optimizer failure mode. Unlike a phase they are never sequenced by the orchestrate DAG — they reach the optimizer as injected read-context, at ./guidance/reasoning/<skill>/, plus native placement in the agent's own skills dir (.claude/skills/…) so a headless CLI auto-loads them.

One skill, mechanism-probe, loaded at the proposal step. The failure mode it counters is the one this framework keeps paying for: the optimizer reads a few traces, recognizes a familiar shape, and ships the first plausible edit — another prose rule for a rule the agent already skips, a retuned threshold, a reworded docstring. It asks one question while the edit is still cheap to throw away: could this whole proposal be replaced by changing one existing value or restating one existing rule? If yes, it is a knob.

I did not add a second first-principles skill. Nothing loads it and nothing would; a registered skill with no caller is a knob of its own. Add it when a second named failure mode actually needs it.

Whether the gate is advisory or enforcing

Advisory. Nothing in this PR can reject a candidate. The exact honest wording, verbatim from the prompt every optimizer receives:

How this is judged: the declaration is RECORDED per candidate, not enforced — a missing or knob-shaped declaration does NOT reject your edit, and a well-declared one does not get it accepted. The val significance gate remains the only thing that accepts or rejects. Declare it anyway: an edit you cannot state a mechanism and an observable for is the edit that historically wasted the iteration.

Why not enforcing: "is this a mechanism or a knob?" is a judgement no regex can make. A heuristic strict enough to reject knobs would also reject a real one-line in-body guard, and a false rejection discards a genuine improvement invisibly — nobody sees the gain that never happened. So this adopts #129's resolution exactly: advisory at the prompt, hard at the val gate. RUN.md and docs/ARCHITECTURE.md say "recorded, never enforced", not "rejects low-quality proposals" — the #222 lesson about not claiming enforcement nothing enforces.

What is precisely checkable is the declaration: presence of three named fields (Mechanism: / Hypothesis: / Expected observable:) in the PROCESS.md the optimizer already writes. cap_evolve.proposal_quality parses those and logs a proposal_quality event per candidate, surfaced in the dashboard's existing annotations stream.

False-rejection probe

The test that matters, pinned from both sides:

  • test_a_genuine_mechanism_proposal_is_not_rejected — a declared, genuinely-improving edit is accepted, and the gate reason contains none of mechanism / knob / declar / proposal quality.
  • test_an_undeclared_proposal_is_also_not_rejected — the bare mock edit, with no declaration at all, is also accepted on its val delta. A missing declaration is a signal, not a verdict.

mechanism-probe's own check.py carries the same probe as a behavioral contract.

LLM calls

Zero. Pure stdlib regex over a markdown file — #205's rule that every auxiliary step in core is pure Python is preserved, and test_the_gate_makes_no_model_call pins it (no anthropic/openai/requests/urllib/aux_model/subprocess in the module). No aux_model tier needed.

Composed-prompt measurement

Routed through #222's single shared cap_instructions — no second cap. test_the_bar_is_capped_by_the_shared_cap_not_a_second_one asserts _augment_instructions applies cap_instructions exactly once.

rendered template + #222 dead-ends (200 rejections) + #140 bar  :  24416
  of which #140's PROMPT_BLOCK                                  :   1320
  of which #222's dead-end block @ 200 rejections               :   2251
+ #219's INSIGHTS pointer bullet                                :    463
+ #221's diversify block (39 lineages -> bounded to 6)          :    887
-------------------------------------------------------------------------------
COMPOSED TOTAL (#219 + #221 + #222 + #140)                      :  25766
CAP (MAX_INSTRUCTIONS_CHARS)                                    :  60000
headroom                                                        :  34234  (42.9% of cap)

Truncation never silently drops a whole block. The block is 1,320 chars and sits LAST, inside the kept 30% tail (18 KB at the default ceiling), next to #222's constraints. test_an_overflowing_prompt_keeps_the_bar_whole crosses the bound (the #219 lesson — a pinning test whose fixture never overflows proves nothing): it feeds 110,000 chars through the real _augment_instructions and asserts the composed output is ≤ cap, the elision notice is present, and both #140's and #222's blocks survive entire.

Per-algorithm evidence

Real cap-evolve run on examples/toy_calc with the mock optimizer, zero API cost, all three deterministic algorithms. diff -r against source is rc=0 for every one:

algorithm workdir diff -r vs source composed prompt bar in prompt proposal_quality logged
hill-climb cand_0001 rc=0 (identical) 22,265
gepa gepa_0001 rc=0 (identical) 23,634
skillopt so_e01s01 rc=0 (identical) 22,793

No-leak proof

grep for each sealed test id across every file in each optimizer workdir: 0 files for all three algorithms. test_no_test_split_id_reaches_any_injected_workdir_file pins it, and asserts the test-id list is non-empty first so the probe cannot be vacuous.

The injected subtree is also invisible to the snapshot / eval-cache hash / GEPA component list — guidance/ is already in #199's INJECTED_DIRS, and test_the_reasoning_skill_is_not_mistaken_for_a_capability_edit proves the hash and component list are unchanged by its presence.

Expected merge order

#199#212#211#219 / #221 / #222this. This branch is cut from origin/feat/issue-129-failure-memory because it must route through #222's cap_instructions. #219 and #221 touch _augment_instructions / the same prompt tail; whichever lands second resolves a small conflict in that one function. #213's skill lint should land before or with this so the new skill is linted in CI from day one (it already passes).

Verification

$ PYTHONPATH=core python -m pytest core/tests -q
240 passed in 103.91s

214 on the #222 base + 19 new + 7 test_dashboard_launch (which passed here). 0 failed.

$ python -m compileall -q core/cap_evolve core/tests skills
rc=0

$ python skills/_registry/build_manifest.py skills
wrote skills/_registry/manifest.json (21 skill(s))
  reasoning: mechanism-probe

$ python skills/_registry/lint_skills.py skills      # #213's lint
skill authoring lint — 21 skill package(s)
  # zero errors AND zero advisories for reasoning/mechanism-probe
  # the 3 remaining errors (capabilities/tools body size, using-cap-evolve XML tags)
  # are pre-existing on this base and are exactly what #213 fixes on its own branch.

$ for f in skills/*/*/scripts/check.py; do ...; done
rc=0 ok=True   × 21 / 21   (including skills/reasoning/mechanism-probe)

Fail-before / pass-after — test file present, implementation stashed:

15 failed, 3 passed in 9.80s

then restored: 19 passed.

Osher Elhadad added 12 commits July 30, 2026 00:49
…ll-climb

The two flagship algorithms ran blind. `_inject_optimizer_context` was called only
from `run_step` (the hill-climb path): GEPA bypasses `run_step` entirely, so its
optimizer got no `./trajectories/`, no `./guidance/<cap>/`, no native-skill
injection and a hand-rolled 1.4 KB prompt with no capability brief. `skillopt_loop`
had no capability/optimizer parameters and called `_focus_instructions` bare, so
CAP_BRIEF was empty and the PARALLEL note always claimed a sequential optimizer.
On the CLI side `--capabilities --instructions-file --bench-repo --optimizer-name
--capability-sources --target-model --target-profile-file` were gated behind
`algorithm_name == "hill-climb"` and silently dropped for the other two.

Adds `core/cap_evolve/optimizer_context.py` — the one seam every algorithm routes
through: `OptimizerContext` (the per-run bundle + its argparse flag set), `inject`
(the file side) and `render_instructions` (the prompt side). hill-climb, gepa and
skillopt now all take `ctx=` and produce identical context; the CLI passes the
flags unconditionally to all three (agent-driven evograph/agent-optimize excluded
explicitly, not per-flag).

GEPA's `./trajectories/` is scoped to the parent's minibatch tag, so it holds the
verbatim untruncated rollouts REFLECTION.md only excerpts. `INJECTED_DIRS/NAMES`
gives the snapshot ignore-list, GEPA's component list and the eval-cache hash one
shared definition, so the newly injected read-context cannot become an editable
component or bust the cache.

Closes #109
…joint failure index

Review fixes for PR #199 (issue #109). Three blocking findings.

1. GEPA's cross-iteration history channel was permanently EMPTY.
   _parent_map / _build_ledger / _build_runmap filtered `kind == "step"`, but GEPA
   bypasses run_step and emits `gepa_val_gate` — so its LEDGER.md, RUNMAP.md and
   prior_iterations/ stayed empty while the prompt instructed the optimizer to read
   them, and it was told its best candidate was `seed` even after an accept.
   dashboard.py already special-cased all three kinds, so the correct set was known
   in-repo. Factored into ONE definition, rundir.ITERATION_EVENT_KINDS +
   RunDir.iteration_events(), now read by all four consumers (dashboard, LEDGER,
   RUNMAP, prior_iterations) so a fourth kind cannot desync a fifth consumer.
   gepa_loop also publishes the running best on each accept, not only at loop exit.
   #128 (persist insight), #129 (failure memory) and #130 (plateau detection) all
   read exactly this channel — it works for GEPA now.

2. Wrong trajectories on cache hits (regression introduced by this PR).
   The new `tag=` pin fell through to the best/seed/whole-dir chain when a cached
   minibatch persisted no rollout files, handing the optimizer a previous iteration's
   traces (including mb_c_* child rollouts) while the prompt claimed they were "the
   SAME minibatch VERBATIM". The pin is now honoured or omitted LOUDLY: the stale dir
   is removed, an optimizer_context_warning is logged, and GEPA's prompt block states
   the minibatch came from the cache with no traces rather than claiming a dir that
   holds someone else's data. Adjacent to #111 (GEPA eval cache drops output/trace);
   this fix does not depend on it — #111 makes trajectories available on a cache hit,
   this makes the absence honest either way.

3. Failure index was always "of 0 tasks" for SkillOpt.
   render_instructions received a val SplitResult narrowed by train ids — disjoint by
   construction — so the index was always empty, and this PR newly routed the
   slow/meta update through the same shape. Fixed at the root in
   _focus_instructions: zero overlap between focus_ids and the scored result no longer
   filters, and the prompt says why. The seam parameter is renamed `current` ->
   `scored_result` so the invariant is visible at every call site instead of only in a
   docstring.

Non-blocking: cache.py no longer mutates module-level sets at import time (plain
constant expression, no import-order dependence; same for harness._SNAPSHOT_IGNORE);
a global MAX_INSTRUCTIONS_CHARS cap bounds the assembled prompt (every block was
bounded, the sum was not); ARCHITECTURE.md names the seam and the shared event-kind
set, and all three algorithm SKILL.mds document the context they receive; the
render_instructions docstring records that `extra=` is for algorithm-specific tails
and shared blocks belong in the body (#128/#129 need no signature change).

Tests: +7 (three blocking regressions, the prompt cap, and a real
--target-profile-file fixture exercised end to end on all three algorithms).
199 passed, 0 failed.
…ean (#110)

GEPA's candidate snapshots were dirty: both `run_dir.snapshot()` calls in gepa.py
omitted `ignore=`, so every accepted candidate carried FOCUS.md, REFLECTION.md,
LEDGER.md, JOURNAL.md and RUNMAP.md alongside the capability — 8 files / 32 KB where
hill-climb stored 3 files, and every dashboard candidate-vs-parent diff showed scratch
churn instead of the real edit.

Root cause is not the missing `ignore=` argument, it is that the scratch-name list was
copy-pasted into four modules and desynced. FOCUS.md/REFLECTION.md were already in
cache._IGNORE_NAMES and gepa._NON_COMPONENT but never in harness._SNAPSHOT_IGNORE, so
even after PR #199 made _SNAPSHOT_IGNORE derived from optimizer_context.INJECTED_*
(which fixed LEDGER/JOURNAL/RUNMAP), GEPA snapshots still leaked GEPA's own scratch.
Same class of bug as #109's `kind == "step"` filter and #189's counts.

Fixed once: rundir.SCRATCH_NAMES is the single definition, at the bottom of the import
graph, read by all four consumers — harness._SNAPSHOT_IGNORE, cache._IGNORE_NAMES,
gepa._NON_COMPONENT and skillopt._SCAFFOLDING (which still had the literal inline,
twice). A newly-injected scratch file now lands in all four automatically.
INSTRUCTIONS.md/PROCESS.md stay deliberately OUT of SCRATCH_NAMES: they are snapshotted
for explainability and filtered at diff time only.

Verified end to end on examples/toy_calc with the mock optimizer: a GEPA candidate went
from 8 files / 32 KB to 3 files / 12 KB and now matches hill-climb's snapshot contents
exactly (INSTRUCTIONS.md, PROCESS.md, prompt.txt) — 5 stray files per accepted
iteration eliminated.

Tests: +3 (the shared-constant invariant, plus an end-to-end snapshot-cleanliness check
parameterized over gepa and hill-climb so the two can't diverge again). Both fail on
origin/main AND on top of #199; hill-climb passes throughout. 182 passed, 0 failed.
compileall core skills clean. Zero new runtime deps.

Closes #110
…ache docstring

The issue's "memory.py is write-only" premise is only PARTLY right. The
render/entries API and the note/impact fields ARE dead, but the jsonl files
themselves have a live reader the issue told us to verify: the dashboard's
GET /api/runs/{id}/memory (dashboard/backend/capevolve_dashboard/memory.py),
which feeds the Memory panel and the Insights "dead ends" grouping. So the
WRITES stay; only the genuinely-unread surface goes.

Removed (zero readers, proven by grep):
  - RejectedMemory.render / .entries, History.render / .entries, _render_impact,
    _store_impact — the prompt-facing API. LEDGER/JOURNAL/RUNMAP replaced it.
  - the `note=` and `impact=` kwargs and the `note`/`broke`/`fixed` record fields.
  - harness._latest_journal_note — its only caller was that dead `note=`.
  - the per-iteration _candidate_task_impact call in run_step, which existed only
    to populate those dead fields (re-read rollouts from disk every iteration).
    The LEDGER and _reconcile_journal paths keep their own computations.
  - the unused `rejected` / `history` params on _augment_instructions and
    _build_ledger.

_init_memory_store is untouched, so PR #204's algorithm-label stamp and the
dashboard badge are unaffected.

cache.py's docstring claimed wiring into evaluate_candidate was "OFF by default
and gated behind a flag (see maybe_cached_score)" — no such wiring, flag, or
function exists. Replaced with what the cache actually does: GEPA-only, consumed
solely by gepa._eval_minibatch; evaluate_candidate always pays full price.

Test change: test_rejected_memory_roundtrip_and_render tested the removed
render(); replaced by test_memory_jsonl_record_shape_matches_dashboard_contract,
which pins the exact keys the dashboard reads — the contract that actually
matters. Net test count unchanged.

Closes #114
…hot + cache hash

Review fix for #211. The four-way unification made harness._SNAPSHOT_IGNORE — the
one DESTRUCTIVE consumer — take the full union. Three of the added names
(MEMORY.md, STATE.md, REJECTED.md) have no live writer in core/, so their only
real-world referent is a capability file that shares the name, and snapshot()
silently deleted it from the candidate, from every descendant iteration, and
WITHOUT busting the eval-cache key (cache ignored the same name) — a stale hit on
a mutilated candidate. Regression vs main.

- rundir: SCRATCH_NAMES (live writers) vs LEGACY_SCRATCH_NAMES (no writer,
  filter-only) + NON_CAPABILITY_NAMES union. Destructive consumer takes the
  subset; every read-side filter takes the union.
- rundir.snapshot: root-anchored ignore callable. shutil.ignore_patterns matches
  by basename at EVERY depth; every entry is a root-level framework injection, so
  a nested src/prompts/STATE.md can no longer be caught.
- cache.hash_candidate_dir: root-anchored too, so deleting a nested colliding
  capability file DOES change the key (closes the stale-hit hazard at the root).
- dashboard._DIFF_SKIP + harness._CAP_DIFF_SKIP now derive from
  NON_CAPABILITY_NAMES — they are read-side filters asking the same question, and
  were the last hardcoded copies (review finding #2).
- test_gepa: pin the live/legacy split and that all five read-side filters equal
  the union; new test_snapshot_ignore_excludes_legacy_names_and_is_root_anchored;
  drop the IndexError-prone parts[] check subsumed by the exact-set assert.
…mory->prompt framing

Review fixes for #212:

1. cache.py — restore main's intro paragraph verbatim (the sentence #211
   rewrites) and confine the correction to a separate "Scope: GEPA only"
   paragraph, so the #211 conflict is a single textual hunk whose wrong
   resolution can no longer restore the false maybe_cached_score line — that
   line's removal now auto-merges outside the conflict region.
2. test_w1_engine.py — new guard pinning that no doc under core/ or skills/
   cites maybe_cached_score, so a revert of the docstring fix fails a test.
3. skillopt SKILL.md / concepts.md / skillopt.py — the rejected/history jsonl
   are dashboard audit records, write-only, never prompt input.
4. MemoryPanel.tsx — drop the "do-not-re-propose" framing this PR disproved.
A filter may legitimately add its own read-context names (post-#199 the cache and
component lists fold in optimizer_context.INJECTED_NAMES); the invariant that
matters is that none of them DROPS a shared name.
…ts (#129)

Rejected candidates were persisted to rejected.jsonl (audit + the dashboard's
"what not to try" panel) but nothing put them in a proposal prompt, so the
optimizer could — and demonstrably did — re-propose an approach the gate had
already killed, burning a full-val eval on a known dead end each time.
RUN.md nonetheless claimed "rejected approaches are remembered and never
re-proposed": false on both halves. This closes the feature gap and the
honesty gap together.

What changed
- harness.approach_signature(parent_dir, cand_dir) — a stable, compact
  signature of WHAT an edit changed, built from the capability diff
  (whitespace-collapsed added/removed lines per touched file). Cosmetic
  variants of the same idea collapse to one signature; a no-op edit yields "".
- harness.dead_end_constraints(run_dir) — the "ALREADY TRIED & REJECTED"
  block: deduped signature + gate reason + a repeat count, with an explicit
  "do not re-propose, and if you revisit one, state in PROCESS.md what is
  materially different" instruction.
- Wired into _augment_instructions, the ONE function whose output reaches the
  optimizer prompt (#114) and which all three algorithms route through — so
  hill-climb, GEPA and SkillOpt get it with no per-algorithm plumbing.
- RejectedMemory.add gains an optional `approach` field; every rejection site
  (run_step, GEPA's local + val gates, GEPA's two merge gates) now records it.

Bounding (zero LLM calls — pure Python, per PR #205)
- <= 12 most-recent DISTINCT approaches, signature <= 300 chars (capped on
  write AND on read), reason <= 200 chars -> block stays ~1 KB and is
  provably < 8 KB even with 50 long rejections.
- optimizer_context.cap_instructions is extracted from render_instructions so
  the FINAL assembled prompt — not just the rendered half — is held under
  MAX_INSTRUCTIONS_CHARS (60k). Previously these cross-iteration blocks were
  appended after the cap had already been applied.

Enforcement is ADVISORY, stated as such
The optimizer is a black-box agent CLI, so cap-evolve cannot forbid it from
re-emitting an edit. What is HARD is the val gate — a re-proposed dead end is
still rejected, and the repeat is counted in the block ("re-proposed 4x").
RUN.md and docs/COMPARISON.md now say exactly this instead of implying a
guarantee.

Drive-by root-cause fix: one scratch-file list, four consumers
Building the signature exposed that cache._IGNORE_NAMES, gepa._NON_COMPONENT,
harness._CAP_DIFF_SKIP and dashboard._DIFF_SKIP each kept their OWN copy of
"framework scratch, not capability", and they had drifted — only GEPA's knew
about FOCUS.md/REFLECTION.md. So a GEPA candidate's "capability diff" (shown
in the dashboard, RUNMAP and prior_iterations) reported its reflective scratch
as a real edit. Unified as optimizer_context.SCRATCH_NAMES.

Dashboard: the Insights dead-ends panel now shows the rejected edit signature
under each reason (same data, one new field).

Closes #129
… edit (#129 review)

Review of #222 found the constraint block was actively misdirecting on every REAL agent
optimizer: it told the optimizer "you already tried this, re-proposed 2x" about approaches
it had never proposed. Four blocking fixes, all reproduced fail-before/pass-after.

B1 — the signature was dominated by framework-injected read-context. A capability diff
compares a SNAPSHOT parent (INJECTED_* already stripped by _SNAPSHOT_IGNORE) against the
LIVE workdir (not stripped), so every injected CLAUDE.md / .claude/skills/<x>/SKILL.md read
as a capability ADDITION, sorted to the front, and truncated the real edit away entirely.
_CAP_DIFF_SKIP omitted INJECTED_NAMES and _capability_files filtered a hardcoded 3-of-9
subset of INJECTED_DIRS. Both sets are now derived, never enumerated — in harness,
dashboard and skillopt alike. Only `mock` (no registry skills_dir) hid this, which is why
13/13 tests passed over a broken path; the new test runs run_step with
optimizer_name="claude-code".

B2 — head-first truncation made the signature not a function of the edit: two different
edits sharing a long prefix (any realistic SKILL.md or system prompt) collapsed to one
signature. Overflow now closes with a sha256 digest of the whole normalized body, so it
stays stable under cosmetic variation AND distinct whenever the bytes differ.

B3 — eviction was by FIRST appearance, so the single most predictive row (a dead end the
optimizer JUST re-proposed) was dropped while newer one-offs were kept, contradicting the
block's own "12 most recent" wording. A repeat now requeues its row.

B4 — skillopt._changed_components was a fifth, pre-drift copy of the scratch list. It now
derives from the one shared definition like the other four.

SCRATCH_NAMES end state (rebased onto #211): rundir.SCRATCH_NAMES /
LEGACY_SCRATCH_NAMES / NON_CAPABILITY_NAMES is the sole definition, split by OPERATION so
the one destructive consumer never takes a retired name; optimizer_context.SCRATCH_NAMES is
deleted. The five read-side filters compose NON_CAPABILITY_NAMES with INJECTED_NAMES/DIRS.

Also: control + bidi-override chars stripped from the signature once (N3); the row shows
"(latest <cid>)" so a repeat count cannot be misread as the first candidate's (N2); a
frontend test for the new `approaches` field (N4); N5 noted with its upgrade path; the
function-body cap_instructions import, the <8 KB bound comment and the 70/30 tail-slice
docstring corrected (nits).
…ality declaration (#140)

Two halves, deliberately different in epistemic standing.

1. A new "reasoning" skill component: tiny skills loaded at ONE step to counter ONE
   named optimizer failure mode, reaching the optimizer as injected read-context rather
   than as a sequenced phase. The first is `mechanism-probe` at the proposal step,
   countering the failure mode this framework keeps paying for: skipping the analysis and
   shipping a plausible one-line knob tweak. It rides #199's shared `inject` seam — one
   `copytree` — so hill-climb, GEPA and SkillOpt all get it, byte-identical, plus native
   placement in the agent's own skills dir.

2. The three-field proposal declaration (mechanism / hypothesis / expected observable),
   seeded into the PROCESS.md the optimizer already writes, parsed by
   `cap_evolve.proposal_quality` and recorded per candidate as a `proposal_quality`
   event that the dashboard's annotations stream surfaces.

ADVISORY, and the wording says so. "Is this a mechanism or a knob?" is a judgement no
regex can make: a heuristic strict enough to reject knobs would also reject real one-line
mechanism fixes, and a false rejection discards a genuine improvement invisibly. So the
bar lives in the prompt where it shapes the proposal, and the hard decision stays on the
val significance gate — the same split #129 settled on. Nothing here can reject a
candidate; the false-rejection probe pins that, from both sides (a declared genuine
improvement and an undeclared one are both accepted on their val delta alone).

Zero LLM calls: pure stdlib parsing, per #205. Zero new runtime deps.

The prompt block routes through #222's single shared `cap_instructions`, not a second
private cap, and is short enough (1,320 chars) to live in the kept 30% tail alongside
#222's dead-end constraints — the overflow test forces a real overflow and asserts both
blocks survive whole rather than being cut mid-list. Composed with #219's INSIGHTS
pointer and #221's diversify block the prompt measures 25,766 / 60,000 chars (42.9%).

Fail-before/pass-after: 15 of the 19 new tests fail with the test file present and the
implementation stashed.
Copilot AI review requested due to automatic review settings July 30, 2026 21:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@skillberry-bot skillberry-bot added algorithm Optimization algorithms: GEPA / SkillOpt / hill-climb enhancement New feature or request dashboard Dashboard backend/frontend labels Jul 30, 2026
@skillberry-bot

Copy link
Copy Markdown
Contributor

🏷️ Automatic Labeling

I've analyzed this pull request and added the following labels:

  • algorithm - enhancement - dashboard - algorithm - enhancement - dashboard - observability

These labels were selected based on the PR title, description, and changed files. If you believe any labels are incorrect or missing, feel free to adjust them manually.

@skillberry-bot skillberry-bot added the observability Live run visibility, logging, tracing label Jul 30, 2026
if "optimizer_context_warning" in json_line and "mb_p_0007" in json_line]
assert warn, "unmet trajectory pin failed SILENTLY (no warning event)"
assert "OMITTED" in warn[0]
assert kinds == kinds # no-op; keeps the reader exercised without asserting count
Comment on lines +1039 to +1044
"**Constraint:** do NOT propose any of the above again, and do not propose a "
"cosmetic variation of one (same text, different wording/placement) — it shares "
"the same hidden assumption and will fail the same way. If you believe a rejected "
"direction is still right, you MUST state in `PROCESS.md` what is materially "
"different this time and which specific lesson above it counters. Otherwise pick "
"a genuinely different hypothesis.",
import tempfile
from pathlib import Path

import _bootstrap # noqa: F401
import sys
from pathlib import Path

import _bootstrap # noqa: F401
@OsherElhadad

Copy link
Copy Markdown
Collaborator Author

🔬 Evidence

All commands run in /tmp/wt-140 — a worktree on feat/issue-140-reasoning-skills, cut from origin/feat/issue-129-failure-memory (= #222, which already carries #199 + #212 + #211). Python is /tmp/ce-venv/bin/python, always with PYTHONPATH=/tmp/wt-140/core. Every optimizer invocation is the deterministic mock optimizer on examples/toy_calczero API cost, zero model calls.


1. Baseline on the #222 base, before my change

$ cd /Users/osherelhadad/Documents/capevo/cap-evolve && git fetch origin
$ git worktree add /tmp/wt-140 -b feat/issue-140-reasoning-skills origin/feat/issue-129-failure-memory
Preparing worktree (new branch 'feat/issue-140-reasoning-skills')
HEAD is now at 6d6cc52d fix(algorithm): make the rejected-approach signature reflect the real edit (#129 review)

$ PYTHONPATH=/tmp/wt-140/core python -m pytest core/tests -q --ignore=core/tests/test_dashboard_launch.py
214 passed in 90.17s (0:01:30)

2. Real end-to-end through the actual CLI — all three deterministic algorithms

Each is a genuine cap-evolve run with algorithm_skill swapped in the spec. Shows the reasoning skill reaching the optimizer's workdir (find + diff -r rc=0 against source, as #213 and #185 did), the composed prompt size, the bar's presence, the logged declaration, and the sealed-split grep.

==============================================================
### cap-evolve run --algorithm hill-climb   (mock optimizer, $0)
==============================================================
  "iterations": 1,
  "dashboard": ".capevolve/run_demo/dashboard.html"
}

--- reasoning skill in each optimizer workdir:
work/cand_0001/guidance/reasoning/mechanism-probe/SKILL.md
    diff rc=0 (identical) : cand_0001
    prompt    22265 chars; bar present: 1
--- proposal_quality events:
"kind": "proposal_quality", "candidate": "cand_0001", "declared": false, "missing": ["mechanism", "hypothesis", "observable"], "mechanism": "", "hypothesis": "", "observable": "", "enforcement": "advisory"
--- sealed test ids leaked into work/ :
    test ids = [a8 a7]
    files in work/ containing a test id: 0
==============================================================
### cap-evolve run --algorithm gepa   (mock optimizer, $0)
==============================================================
  "iterations": 1,
  "dashboard": ".capevolve/run_demo/dashboard.html"
}

--- reasoning skill in each optimizer workdir:
work/gepa_0001/guidance/reasoning/mechanism-probe/SKILL.md
    diff rc=0 (identical) : gepa_0001
    prompt    23634 chars; bar present: 1
--- proposal_quality events:
"kind": "proposal_quality", "candidate": "gepa_0001", "declared": false, "missing": ["mechanism", "hypothesis", "observable"], "mechanism": "", "hypothesis": "", "observable": "", "enforcement": "advisory"
--- sealed test ids leaked into work/ :
    test ids = [a8 a7]
    files in work/ containing a test id: 0
==============================================================
### cap-evolve run --algorithm skillopt   (mock optimizer, $0)
==============================================================
  "iterations": 1,
  "dashboard": ".capevolve/run_demo/dashboard.html"
}

--- reasoning skill in each optimizer workdir:
work/so_e01s01/guidance/reasoning/mechanism-probe/SKILL.md
    diff rc=0 (identical) : so_e01s01
    prompt    22793 chars; bar present: 1
--- proposal_quality events:
"kind": "proposal_quality", "candidate": "so_e01s01", "declared": false, "missing": ["mechanism", "hypothesis", "observable"], "mechanism": "", "hypothesis": "", "observable": "", "enforcement": "advisory"
--- sealed test ids leaked into work/ :
    test ids = [a8 a7]
    files in work/ containing a test id: 0

diff -r rc=0 (identical) for all three; files in work/ containing a test id: 0 for all three.


3. The same three algorithms driven directly through the library loops

Adds the verbatim prompt text of the gate and the full proposal_quality event.


==============================================================================
### hill-climb  workdir=cand_0001
==============================================================================
--- find guidance/reasoning:
    guidance/reasoning/mechanism-probe/SKILL.md
    guidance/reasoning/mechanism-probe/meta.yaml
    guidance/reasoning/mechanism-probe/scripts/_bootstrap.py
    guidance/reasoning/mechanism-probe/scripts/check.py
    guidance/reasoning/mechanism-probe/scripts/run.py
--- diff -r vs source: rc=0 (identical)
--- composed prompt: 22893 chars / cap 60000 (38.2%)
    [x] #140 bar
    [x] #140 honest wording
    [x] #222 dead-ends
    [x] skill pointer
--- the bar, verbatim from the prompt:
    ## Proposal quality — declare the MECHANISM, not a knob (advisory bar)
    Read `./guidance/reasoning/mechanism-probe/SKILL.md` BEFORE you decide what to edit. It counters the failure mode this framework keeps paying for: skipping the analysis and shipping a plausible one-line tweak.
    Fill the **Proposal declaration** block in `./PROCESS.md` — three fields:
    - `Mechanism:` what in the system now behaves differently, and WHY that changes the outcome. A *knob* restates an existing rule or retunes a value the agent already ignores; a *mechanism* changes what is structurally possible (an in-code guard, a computation, a new/composite tool, a narrowed decision rule).
    - `Hypothesis:` which failure cluster this fixes and why it generalizes beyond the exact failing inputs.
    - `Expected observable:` the concrete change you expect in the NEXT iteration's trajectories if the hypothesis holds — something a reader could check.
    **How this is judged:** the declaration is RECORDED per candidate, not enforced — a missing or knob-shaped declaration does NOT reject your edit, and a well-declared one does not get it accepted. The val significance gate remains the only thing that accepts or rejects. Declare it anyway: an edit you cannot state a mechanism and an observable for is the edit that historically wasted the iteration.
    
--- proposal_quality events: [{"t": 1785447002.5906901, "kind": "proposal_quality", "candidate": "cand_0001", "declared": false, "missing": ["mechanism", "hypothesis", "observable"], "mechanism": "", "hypothesis": "", "observable": "", "enforcement": "advisory"}]
--- sealed test ids ['a8', 'a7'] in workdir files: 1 hits [('RUNMAP.md', 'a7')]

==============================================================================
### gepa  workdir=gepa_0001
==============================================================================
--- find guidance/reasoning:
    guidance/reasoning/mechanism-probe/SKILL.md
    guidance/reasoning/mechanism-probe/meta.yaml
    guidance/reasoning/mechanism-probe/scripts/_bootstrap.py
    guidance/reasoning/mechanism-probe/scripts/check.py
    guidance/reasoning/mechanism-probe/scripts/run.py
--- diff -r vs source: rc=0 (identical)
--- composed prompt: 23900 chars / cap 60000 (39.8%)
    [x] #140 bar
    [x] #140 honest wording
    [x] #222 dead-ends
    [x] skill pointer
--- the bar, verbatim from the prompt:
    ## Proposal quality — declare the MECHANISM, not a knob (advisory bar)
    Read `./guidance/reasoning/mechanism-probe/SKILL.md` BEFORE you decide what to edit. It counters the failure mode this framework keeps paying for: skipping the analysis and shipping a plausible one-line tweak.
    Fill the **Proposal declaration** block in `./PROCESS.md` — three fields:
    - `Mechanism:` what in the system now behaves differently, and WHY that changes the outcome. A *knob* restates an existing rule or retunes a value the agent already ignores; a *mechanism* changes what is structurally possible (an in-code guard, a computation, a new/composite tool, a narrowed decision rule).
    - `Hypothesis:` which failure cluster this fixes and why it generalizes beyond the exact failing inputs.
    - `Expected observable:` the concrete change you expect in the NEXT iteration's trajectories if the hypothesis holds — something a reader could check.
    **How this is judged:** the declaration is RECORDED per candidate, not enforced — a missing or knob-shaped declaration does NOT reject your edit, and a well-declared one does not get it accepted. The val significance gate remains the only thing that accepts or rejects. Declare it anyway: an edit you cannot state a mechanism and an observable for is the edit that historically wasted the iteration.
    
--- proposal_quality events: [{"t": 1785447003.814673, "kind": "proposal_quality", "candidate": "gepa_0001", "declared": false, "missing": ["mechanism", "hypothesis", "observable"], "mechanism": "", "hypothesis": "", "observable": "", "enforcement": "advisory"}]
--- sealed test ids ['a8', 'a7'] in workdir files: 1 hits [('RUNMAP.md', 'a7')]

==============================================================================
### skillopt  workdir=so_e01s01
==============================================================================
--- find guidance/reasoning:
    guidance/reasoning/mechanism-probe/SKILL.md
    guidance/reasoning/mechanism-probe/meta.yaml
    guidance/reasoning/mechanism-probe/scripts/_bootstrap.py
    guidance/reasoning/mechanism-probe/scripts/check.py
    guidance/reasoning/mechanism-probe/scripts/run.py
--- diff -r vs source: rc=0 (identical)
--- composed prompt: 23421 chars / cap 60000 (39.0%)
    [x] #140 bar
    [x] #140 honest wording
    [x] #222 dead-ends
    [x] skill pointer
--- the bar, verbatim from the prompt:
    ## Proposal quality — declare the MECHANISM, not a knob (advisory bar)
    Read `./guidance/reasoning/mechanism-probe/SKILL.md` BEFORE you decide what to edit. It counters the failure mode this framework keeps paying for: skipping the analysis and shipping a plausible one-line tweak.
    Fill the **Proposal declaration** block in `./PROCESS.md` — three fields:
    - `Mechanism:` what in the system now behaves differently, and WHY that changes the outcome. A *knob* restates an existing rule or retunes a value the agent already ignores; a *mechanism* changes what is structurally possible (an in-code guard, a computation, a new/composite tool, a narrowed decision rule).
    - `Hypothesis:` which failure cluster this fixes and why it generalizes beyond the exact failing inputs.
    - `Expected observable:` the concrete change you expect in the NEXT iteration's trajectories if the hypothesis holds — something a reader could check.
    **How this is judged:** the declaration is RECORDED per candidate, not enforced — a missing or knob-shaped declaration does NOT reject your edit, and a well-declared one does not get it accepted. The val significance gate remains the only thing that accepts or rejects. Declare it anyway: an edit you cannot state a mechanism and an observable for is the edit that historically wasted the iteration.
    
--- proposal_quality events: [{"t": 1785447005.03246, "kind": "proposal_quality", "candidate": "so_e01s01", "declared": false, "missing": ["mechanism", "hypothesis", "observable"], "mechanism": "", "hypothesis": "", "observable": "", "enforcement": "advisory"}]
--- sealed test ids ['a8', 'a7'] in workdir files: 1 hits [('RUNMAP.md', 'a7')]

4. Composed-prompt measurement — #219 + #221 + #222 + #140, and the overflow probe

#219's INSIGHTS pointer bullet is taken verbatim from origin/feat/issue-128-persist-insight; #221's diversify block is produced by its own plateau.prompt_block() from origin/feat/issue-130-plateau-detection at its bounded widest (39 exhausted lineages → bounded to 6); #222's dead-end block is rendered live at 200 rejections.

rendered template + #222 dead-ends (200 rejections) + #140 bar  :  24416
  of which #140's PROMPT_BLOCK                                  :   1320
  of which #222's dead-end block @ 200 rejections               :   2251
+ #219's INSIGHTS pointer bullet                                :    463
+ #221's diversify block (39 lineages -> bounded to 6)          :    887
-------------------------------------------------------------------------------
COMPOSED TOTAL (#219 + #221 + #222 + #140)                      :  25766
CAP (MAX_INSTRUCTIONS_CHARS)                                    :  60000
headroom                                                        :  34234  (42.9% of cap)

overflow probe: _augment_instructions(110000 chars) -> 59906 chars; <= cap: True
  #140's block survives WHOLE in the kept tail : True
  #222's dead-end block survives WHOLE too     : True
  elision notice present (nothing silently cut): True

25,766 / 60,000 = 42.9% of the cap, 34,234 chars of headroom. #140's own contribution is 1,320 chars — well inside the kept 30% tail (18 KB), which is why the overflow probe shows both it and #222's block surviving whole. The overflow probe crosses the bound with a real 110,000-char input, not a fixture that never overflows (the #219 lesson).


5. The new test file, verbose — including the false-rejection probe

============================= test session starts ==============================
platform darwin -- Python 3.14.2, pytest-9.1.1, pluggy-1.6.0 -- /private/tmp/ce-venv/bin/python
cachedir: .pytest_cache
rootdir: /private/tmp/wt-140/core
configfile: pyproject.toml
plugins: anyio-4.14.2
collecting ... collected 19 items

core/tests/test_proposal_quality.py::test_a_filled_declaration_parses_all_three_fields PASSED [  5%]
core/tests/test_proposal_quality.py::test_the_seeded_placeholders_count_as_missing PASSED [ 10%]
core/tests/test_proposal_quality.py::test_partial_and_placeholder_values_are_reported_field_by_field PASSED [ 15%]
core/tests/test_proposal_quality.py::test_markup_and_marker_variants_still_parse PASSED [ 21%]
core/tests/test_proposal_quality.py::test_a_missing_process_md_never_raises PASSED [ 26%]
core/tests/test_proposal_quality.py::test_a_genuine_mechanism_proposal_is_not_rejected PASSED [ 31%]
core/tests/test_proposal_quality.py::test_an_undeclared_proposal_is_also_not_rejected PASSED [ 36%]
core/tests/test_proposal_quality.py::test_the_bar_states_it_is_recorded_not_enforced PASSED [ 42%]
core/tests/test_proposal_quality.py::test_the_declaration_is_recorded_per_candidate PASSED [ 47%]
core/tests/test_proposal_quality.py::test_the_declaration_is_surfaced_in_the_dashboard PASSED [ 52%]
core/tests/test_proposal_quality.py::test_hill_climb_gets_the_reasoning_skill PASSED [ 57%]
core/tests/test_proposal_quality.py::test_gepa_gets_the_reasoning_skill PASSED [ 63%]
core/tests/test_proposal_quality.py::test_skillopt_gets_the_reasoning_skill PASSED [ 68%]
core/tests/test_proposal_quality.py::test_the_reasoning_skill_is_not_mistaken_for_a_capability_edit PASSED [ 73%]
core/tests/test_proposal_quality.py::test_the_composed_prompt_is_measured_and_under_the_cap PASSED [ 78%]
core/tests/test_proposal_quality.py::test_an_overflowing_prompt_keeps_the_bar_whole PASSED [ 84%]
core/tests/test_proposal_quality.py::test_the_bar_is_capped_by_the_shared_cap_not_a_second_one PASSED [ 89%]
core/tests/test_proposal_quality.py::test_no_test_split_id_reaches_any_injected_workdir_file PASSED [ 94%]
core/tests/test_proposal_quality.py::test_the_gate_makes_no_model_call PASSED [100%]

============================= 19 passed in 12.52s ==============================

The three that matter for the "risky half":

  • test_a_genuine_mechanism_proposal_is_not_rejectedfalse-rejection probe: a declared, genuinely-improving mechanism edit is ACCEPTED, and the gate reason contains none of mechanism/knob/declar/proposal quality.
  • test_an_undeclared_proposal_is_also_not_rejected — the symmetric half: no declaration at all is also accepted on its val delta.
  • test_the_bar_states_it_is_recorded_not_enforced — pins the honest wording, so nobody can quietly upgrade the claim.

6. Fail-before / pass-after

Test file kept, implementation stashed (git stash -u, then the test file copied back):

$ PYTHONPATH=/tmp/wt-140/core python -m pytest core/tests/test_proposal_quality.py -q
...
E       ImportError: cannot import name 'proposal_quality' from 'cap_evolve'
FAILED core/tests/test_proposal_quality.py::test_a_filled_declaration_parses_all_three_fields
FAILED core/tests/test_proposal_quality.py::test_the_seeded_placeholders_count_as_missing
FAILED core/tests/test_proposal_quality.py::test_partial_and_placeholder_values_are_reported_field_by_field
FAILED core/tests/test_proposal_quality.py::test_markup_and_marker_variants_still_parse
FAILED core/tests/test_proposal_quality.py::test_a_missing_process_md_never_raises
FAILED core/tests/test_proposal_quality.py::test_an_undeclared_proposal_is_also_not_rejected
FAILED core/tests/test_proposal_quality.py::test_the_bar_states_it_is_recorded_not_enforced
FAILED core/tests/test_proposal_quality.py::test_the_declaration_is_recorded_per_candidate
FAILED core/tests/test_proposal_quality.py::test_hill_climb_gets_the_reasoning_skill
FAILED core/tests/test_proposal_quality.py::test_gepa_gets_the_reasoning_skill
FAILED core/tests/test_proposal_quality.py::test_skillopt_gets_the_reasoning_skill
FAILED core/tests/test_proposal_quality.py::test_the_reasoning_skill_is_not_mistaken_for_a_capability_edit
FAILED core/tests/test_proposal_quality.py::test_the_composed_prompt_is_measured_and_under_the_cap
FAILED core/tests/test_proposal_quality.py::test_an_overflowing_prompt_keeps_the_bar_whole
FAILED core/tests/test_proposal_quality.py::test_the_gate_makes_no_model_call
15 failed, 3 passed in 9.80s

(That run predates the dashboard test, hence 18 collected; with it, 19.) After git stash pop: 19 passed.


7. Full suite, including the #200-flaky dashboard test

$ PYTHONPATH=/tmp/wt-140/core python -m pytest core/tests -q
........................................................................ [ 30%]
........................................................................ [ 60%]
........................................................................ [ 90%]
........................                                                 [100%]
240 passed in 99.76s (0:01:39)

240 passed, 0 failed. 214 (base) + 19 (new) + 7 (test_dashboard_launch, which passed on this machine — port 7878 was free).


8. compileall

$ find . -name __pycache__ -type d -exec rm -rf {} +
$ python -m compileall -q core/cap_evolve core/tests skills
compileall rc=0

9. build_manifest.py

$ PYTHONPATH=/tmp/wt-140/core python skills/_registry/build_manifest.py skills
wrote skills/_registry/manifest.json (21 skill(s))
  algorithm: agent-optimize, evograph, gepa, hill-climb, skillopt
  capability: mcp-tool, skill-package, system-prompt, tools
  optimizer: run-optimizer
  orchestrate: orchestrate, using-cap-evolve
  phase: baseline, diagnose, evaluate, finalize, gate, implement-and-check, intake, report
  reasoning: mechanism-probe

21 skills, 0 validation errors. The new reasoning component is added to build_manifest.COMPONENTS, so a typo in it still fails the build loudly.


10. #213's skill authoring lint

lint_skills.py taken verbatim from origin/feat/issue-107-skill-lint-ci and pointed at this tree:

$ python /tmp/lint_skills.py /tmp/wt-140/skills
skill authoring lint — 21 skill package(s) under /private/tmp/wt-140/skills
  ERROR   capabilities/tools: SKILL.md body is 673 lines (>500); split detail into references/ (progressive disclosure)
  ERROR   capabilities/tools: SKILL.md body is ~10934 tokens (>5000); it is a recurring per-session cost — move detail into references/
  ERROR   orchestrate/using-cap-evolve: description must not contain XML tags
  advise  algorithms/evograph: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  algorithms/hill-climb: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  optimizers/run-optimizer: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  orchestrate/using-cap-evolve: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  phases/baseline: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  phases/diagnose: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  phases/finalize: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  phases/gate: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  phases/implement-and-check: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  phases/intake: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
  advise  phases/report: description should say WHEN to use the skill ('Use when …') — it is the primary triggering signal
FAIL — 3 authoring violation(s) in 2 skill(s)

reasoning/mechanism-probe has zero errors and zero advisories — including the "say WHEN" description heuristic, and #213's _manifest_drift path-set check agrees (21 on disk == 21 in manifest.json).

The 3 remaining errors are pre-existing on this base, proved by stashing my change:

$ git stash -u && python /tmp/lint_skills.py /tmp/wt-140/skills | grep ERROR
  ERROR   capabilities/tools: SKILL.md body is 673 lines (>500); split detail into references/ (progressive disclosure)
  ERROR   capabilities/tools: SKILL.md body is ~10934 tokens (>5000); it is a recurring per-session cost — move detail into references/
  ERROR   orchestrate/using-cap-evolve: description must not contain XML tags
  ERROR   discovery: 21 skill package(s) on disk do not match the 20 in manifest.json ...

Those first three are exactly what #213 fixes on its own branch (it splits tools/SKILL.md and rewrites the using-cap-evolve description). Nothing here regresses the lint.


11. Every skill's check.py

$ for f in skills/*/*/scripts/check.py; do (cd $(dirname $f) && python check.py); done
rc=0 "ok": True   skills/algorithms/agent-optimize/scripts/check.py
rc=0 "ok": True   skills/algorithms/evograph/scripts/check.py
rc=0 "ok": True   skills/algorithms/gepa/scripts/check.py
rc=0 "ok": True   skills/algorithms/hill-climb/scripts/check.py
rc=0 "ok": True   skills/algorithms/skillopt/scripts/check.py
rc=0 "ok": True   skills/capabilities/mcp-tool/scripts/check.py
rc=0 "ok": True   skills/capabilities/skill-package/scripts/check.py
rc=0 "ok": True   skills/capabilities/system-prompt/scripts/check.py
rc=0 "ok": True   skills/capabilities/tools/scripts/check.py
rc=0 "ok": True   skills/optimizers/run-optimizer/scripts/check.py
rc=0 "ok": True   skills/orchestrate/orchestrate/scripts/check.py
rc=0 "ok": True   skills/orchestrate/using-cap-evolve/scripts/check.py
rc=0 "ok": True   skills/phases/baseline/scripts/check.py
rc=0 "ok": True   skills/phases/diagnose/scripts/check.py
rc=0 "ok": True   skills/phases/evaluate/scripts/check.py
rc=0 "ok": True   skills/phases/finalize/scripts/check.py
rc=0 "ok": True   skills/phases/gate/scripts/check.py
rc=0 "ok": True   skills/phases/implement-and-check/scripts/check.py
rc=0 "ok": True   skills/phases/intake/scripts/check.py
rc=0 "ok": True   skills/phases/report/scripts/check.py
rc=0 "ok": True   skills/reasoning/mechanism-probe/scripts/check.py

21 / 21 print "ok": true and exit 0, including the new one. mechanism-probe's own check, in full:

{
  "skill": "mechanism-probe",
  "ok": true,
  "problems": [],
  "notes": [
    "run entry exposes main()",
    "all three declared fields parse from PROCESS.md",
    "false-rejection probe: a real mechanism proposal is never rejected",
    "declaration is recorded per candidate as advisory",
    "unfilled placeholders count as missing (no vacuous pass)"
  ]
}
rc=0

Note false-rejection probe: a real mechanism proposal is never rejected — the probe is a shipped behavioral contract, not only a pytest.


12. No sealed-test leak

Per §2 and §3 above: for every algorithm, grep for each sealed test id across every file in the optimizer workdir → 0 hits. test_no_test_split_id_reaches_any_injected_workdir_file pins it in CI and asserts the test-id list is non-empty first, so the probe cannot pass vacuously.

The injected subtree is also invisible to the three read-side filters (guidance/ is already in #199's INJECTED_DIRS):

$ python -m pytest core/tests/test_proposal_quality.py::test_the_reasoning_skill_is_not_mistaken_for_a_capability_edit -q
1 passed

— i.e. hash_candidate_dir(a) == hash_candidate_dir(b) and _components(a) == _components(b) with the skill present in one and not the other. Without that, every candidate would look edited and the eval cache would never hit.


13. Zero LLM calls

$ python -m pytest core/tests/test_proposal_quality.py::test_the_gate_makes_no_model_call -q
1 passed

Asserts none of anthropic, openai, requests, urllib.request, http.client, aux_model, subprocess appear in core/cap_evolve/proposal_quality.py. Pure stdlib re over a markdown file — #205's invariant that every auxiliary step in core is pure Python is intact, and the cost profile of a run is unchanged.


14. Commit authorship

$ git log --format="%h %an <%ae>%n%s" -1
2cb4d1dc Osher Elhadad <Osher.Elhadad@ibm.com>
feat(algorithm): on-demand reasoning skills + an advisory proposal-quality declaration (#140)

$ git log -1 --format=%B | grep -ci "co-authored-by\|generated with"
0
0

Authored and committed as Osher Elhadad <Osher.Elhadad@ibm.com>; no Co-Authored-By and no "Generated with" line.

@OsherElhadad

Copy link
Copy Markdown
Collaborator Author

🔍 Review — PR #246

Verdict: APPROVE WITH NITS.

The central claim holds. I traced every consumer of the proposal_quality signal and re-ran every claim in the evidence section. "Advisory" is true in the code, not just in prose: nothing branches on the parsed declaration, at any acceptance site, in any algorithm. The composed prompt stays capped with all four blocks at their bounded widest and nothing is silently dropped. The skill reaches all three algorithms byte-identically with the cache hash and component list unchanged. Two real defects below, both non-blocking because neither can affect a run's honesty; the first is a broken documented command.


Blocking

None.


Non-blocking

1. skills/reasoning/mechanism-probe/SKILL.md:115 — the documented self-check command cannot run from the optimizer's workdir, and neither can the injected copy.

The SKILL.md tells the optimizer:

python skills/reasoning/mechanism-probe/scripts/run.py --process ./PROCESS.md

The optimizer's cwd is the workdir, which has no skills/ tree — only guidance/reasoning/mechanism-probe/. Reproduced from a real hill-climb workdir:

$ cd .../run_hill/work/cand_0001
$ python skills/reasoning/mechanism-probe/scripts/run.py --process ./PROCESS.md
can't open file '.../work/cand_0001/skills/reasoning/mechanism-probe/scripts/run.py': [Errno 2] No such file or directory

And the path that does exist fails too, because run.py imports cap_evolve.proposal_quality and the bootstrap's upward walk from work/<cid>/guidance/reasoning/mechanism-probe/scripts/ never reaches a core/:

$ python guidance/reasoning/mechanism-probe/scripts/run.py --process ./PROCESS.md
ImportError: cannot import name 'proposal_quality' from 'cap_evolve'

(That import resolved to my editable install of main's core — i.e. the walk found some cap_evolve, just not this branch's. harness.optimizer_from_command (core/cap_evolve/harness.py:455) passes dict(os.environ) through, so CAPEVOLVE_CORE reaches the subprocess only when the caller already had it exported; a plain cap-evolve run does not set it.)

Consequence: the only executable artifact in the new skill is dead on arrival for the agent that is told to run it. The PR's own comment ("the probe's own run.py is what the SKILL.md tells the optimizer to run on its PROCESS.md", core/cap_evolve/harness.py:1271) is the stated reason scripts/ is kept in the injected copy — so the copy exists specifically to serve a command that cannot execute. Every check.py passes because it runs from the repo, where both problems are absent.

Fix (pick one): either change line 115 to the workdir-relative path (python ./guidance/reasoning/mechanism-probe/scripts/run.py --process ./PROCESS.md) and export CAPEVOLVE_CORE in optimizer_from_command's env; or drop scripts/ from the injected copy (add "scripts" to the existing ignore_patterns at core/cap_evolve/harness.py:1281, matching the native copy which already excludes it) and delete the "Checking your own declaration" section. The second is smaller and loses nothing the prompt needs — presence-checking is exactly what the harness already logs.

2. core/cap_evolve/proposal_quality.py:51_field takes the FIRST match in the file, so an appended declaration is shadowed by the seed's placeholder above it.

re.search with re.M returns the earliest match. The seed (core/cap_evolve/harness.py:569-577) writes - Mechanism: <what now behaves DIFFERENTLY...> near the top. An agent that appends a filled block instead of editing in place records as fully undeclared:

fills in place:      missing=['hypothesis', 'observable']   (only the field it edited parses)
appends below seed:  missing=['mechanism','hypothesis','observable']  declared=False
                     -> mechanism=''

Consequence: advisory-only, so no run outcome changes — but the recorded signal is wrong in a plausible-and-common agent behaviour, and the dashboard row then reads "no mechanism declaration" for a candidate that declared one properly. That inverts the one thing the feature exists to observe. It also means the "declared" rate this feature is meant to surface will read artificially low.

Fix: prefer the LAST match (m = None; for m in re.finditer(...): pass), or scope the search to the text after the last ## Proposal declaration heading. Either is a two-line change. Add a test with _PROCESS_SEED + <filled block> as the fixture — the existing test_the_seeded_placeholders_count_as_missing uses the seed alone, so it cannot catch this.

3. core/tests/test_proposal_quality.py:141-172 — the false-rejection probe covers hill-climb only; GEPA's local gate is unguarded by #140's own tests.

I injected an enforcement into GEPA's local gate:

        local_pass = _sum_reward(child_mb) > _sum_reward(parent_mb)
        if not proposal_quality.parse(workdir)["declared"]:
            local_pass = False  # PROBE-INJECTED ENFORCEMENT

All 19 of #140's tests still pass:

$ PYTHONPATH=/tmp/rv-246/core python -m pytest core/tests/test_proposal_quality.py -q
19 passed in 11.65s

It is caught, but by pre-existing tests in a different file (core/tests/test_gepa.py: 5 failures — test_gepa_loop_accepts_and_seals_test, test_per_instance_frontier_used, both merge tests, test_candidate_snapshots_are_clean_for_every_algorithm[gepa]). So the invariant is protected today, incidentally, by tests that would also break for a dozen unrelated reasons. test_gepa_gets_the_reasoning_skill (line 294) asserts injection and the presence of an event; it never asserts the candidate was accepted.

Consequence: if someone later "upgrades" #140 to gate GEPA locally, its own test file green-lights it and the failure surfaces as five confusing GEPA test failures rather than "the advisory gate became enforcing".

Fix: add one assertion to test_gepa_gets_the_reasoning_skill — the undeclared candidate passed the local gate (_events(run_dir, "gepa_local_gate")[-1]["passed"] is True), mirroring the hill-climb probe.


Nits

  • core/cap_evolve/proposal_quality.py:41_PLACEHOLDER_RE's [-–—.]* alternative matches the empty string, so the branch is redundant with the ^\s*...\s*$ anchoring; harmless, but it makes the pattern read as if it rejects more than it does.
  • core/cap_evolve/proposal_quality.py:38expected\s+observable requires "Expected"; a bare Observable: records as missing. Verified. Arguably correct (the seed says "Expected observable"), but it's a brittleness the docstring at line 47 claims tolerance for.
  • One-character values count as declared (Mechanism: adeclared=True). Consistent with "presence only, by design", just worth knowing the signal's floor.

Is "advisory" true in the code?

Yes. Every path traced:

Producers. The only two call sites of record() are core/cap_evolve/harness.py:1508 (in run_step, between update_spent and evaluate_candidate) and core/cap_evolve/gepa.py:638 (after the optimizer returns, before the local-gate eval). Both discard the return value — the call is a bare expression statement, so there is no variable for a later branch to read even by accident. grep -rn "proposal_quality" over the whole repo returns 22 hits and no third producer.

Acceptance sites, each checked:

  • gate.decide (core/cap_evolve/harness.py:1520) — arguments are current_val.reward, cand_val.reward, stderrs, paired_deltas, run_dir, **gate_kwargs. No declaration data in any of them.
  • run_step's accept branch (harness.py:1531) — accepted = decision.accept, then only the no_regression per-task check can flip it. Nothing else.
  • GEPA's local gate (gepa.py:643) — local_pass = _sum_reward(child_mb) > _sum_reward(parent_mb). Rewards only.
  • GEPA's val gate (gepa.py:670, _full_val_gate) — same shape as run_step's.
  • GEPA's merge path (gepa.py:787 _try_merge) — routes to _full_val_gate; no record call, no read.
  • SkillOpt — both the per-step path (skillopt.py:306) and the epoch-boundary slow update (skillopt.py:500) delegate to harness.run_step, so they inherit the same non-branching record and nothing else.
  • plateau (feat(algorithm): plateau/convergence detection with escalation + per-lineage exhaustion #221) — reads iteration_events() only.
  • insights (Durable synthesized priors (INSIGHTS.md) fed to every proposal, all three algorithms #219) / dead-end memory (feat(algorithm): re-inject rejected approaches as optimizer constraints (#129) #222) — rejected.add(...) is fed decision.reason, cand_val.reward and approach_signature(parent_dir, workdir). approach_signature (harness.py:918) is built from _diff_capabilities, which filters _CAP_DIFF_SKIP_DIRS ⊇ INJECTED_DIRS ∋ guidance — so neither the declaration nor the injected skill can enter a dead-end signature.

Event-consumer surface. RunDir.iteration_events() (rundir.py:469) filters on ITERATION_EVENT_KINDS = ("step", "skillopt_step", "gepa_val_gate") (rundir.py:81). proposal_quality is not in that tuple, so the LEDGER (harness.py:746), RUNMAP/prior_iterations (harness.py:846) and _parent_map (harness.py:669) cannot see it. The only reader anywhere is dashboard.py:284, which appends to the read-only diagnoses list.

Wording-pinning test — does it fail on a reword? Yes, and it fails on the substantive claim, not a stray phrase. I rewrote PROMPT_BLOCK's judgement paragraph into an enforcing claim while preserving structure:

- "**How this is judged:** the declaration is RECORDED per candidate, not enforced — ..."
+ "**How this is judged:** the declaration is ENFORCED per candidate — a missing or
+  knob-shaped declaration WILL reject your edit before it reaches evaluation."

4 failed, 15 passed
FAILED test_the_bar_states_it_is_recorded_not_enforced
FAILED test_hill_climb_gets_the_reasoning_skill
FAILED test_gepa_gets_the_reasoning_skill
FAILED test_skillopt_gets_the_reasoning_skill

Four tests, not one — and three of them fail because _assert_probe_in_workdir (line 277) checks the wording in the rendered prompt, not in the constant. That is the right ground truth: it would also catch someone who kept the honest constant but stopped appending it. This is not #244's "Honest limits" failure shape.

Better still, the behavioural half is guarded too. I injected a real enforcement into run_step while leaving the honest wording intact:

    accepted = decision.accept
    if not _q["declared"]:
        accepted = False

FAILED test_an_undeclared_proposal_is_also_not_rejected
    AssertionError: an UNDECLARED proposal was rejectedthe gate is not advisory

So the claim is pinned by wording and by behaviour. The gap is GEPA's local gate (non-blocking #3).

Is the framing honest and useful? Honest: yes, verified above, and stated in the prompt, the SKILL.md, the module docstring and docs/ARCHITECTURE.md:53 in the same terms. No surface overclaims — I grepped docs/, README.md, CHANGELOG.md, RUN.md, CONTRIBUTING.md; the only mention is the ARCHITECTURE.md block, which says "recorded, never enforced". The dashboard is careful in both branches: declared → "declared mechanism: … | expected observable: …", undeclared → "no mechanism declaration (…) — advisory, the val gate still decided this candidate". It rides the existing diagnoses stream, is rendered as plain text in both the static HTML (dashboard.py:1280) and the React Insights.tsx:110, and never sits near an accept/reject badge. reduce_run's whole return passes through redact (dashboard.py:593), so a declaration that quotes a secret is scrubbed.

Useful: the claim "an edit you cannot state a mechanism for is the edit that historically wasted the iteration" is an assertion, not a finding. The SKILL.md gestures at evidence ("Prior runs in this repo lost iterations exactly this way (see any run's LEDGER.md)") but cites no run, no number, no committed artifact. Nothing in-repo measures knob-vs-mechanism edit outcomes, and nothing could — the proposal_quality event this PR adds is the first data that would let you test it, and it has zero rows so far. So: the mechanism is honest and cheap (1,320 chars, ~2% of the prompt budget, zero API calls, zero runtime risk), the rationale is unvalidated, and the feature is the instrument you'd need to validate it. That is a defensible order of operations, but the PR should say "unvalidated hypothesis; this event is how we'd test it" rather than "historically wasted", which reads as though a measurement exists. A one-line honesty edit to the SKILL.md's opening claim would close it.


Composed prompt at the boundary

PR's overflow probe, reproduced exactly:

probe 110000 -> 59906  <=cap True  block whole True  notice True

It genuinely crosses the bound (input 110,000 > cap 60,000 → the elision branch fires, notice present). Not #219's vacuous-fixture shape.

Boundary sweep through the real _augment_instructions:

  in= 57000 out= 59284 <=cap= True whole= True notice=False
  in= 58500 out= 59904 <=cap= True whole= True notice= True   <- first crossing
  in= 58600 out= 59905 <=cap= True whole= True notice= True
  in= 59999 out= 59905 <=cap= True whole= True notice= True
  in= 60000 out= 59905 <=cap= True whole= True notice= True
  in= 60001 out= 59905 <=cap= True whole= True notice= True
  in= 61000 out= 59905 <=cap= True whole= True notice= True

No overshoot at any input size, unlike #219's original 98-char miss. The arithmetic is sound because cap_instructions truncates and inserts the notice in one expression: keep = max_chars - 200 = 59,800; head 41,860 + tail 17,940 + notice 107 = 59,907 ≤ 60,000, with 93 chars of slack. The 200-char reserve genuinely covers the notice.

All four blocks at their bounded widest, simultaneously. I saturated rejected.jsonl with 400 records at _MAX_APPROACH_CHARS/200-char reasons to force #222's block to its ceiling, built #221's diversify block at 6 truncated lineages + "(+33 more)", and #219's INSIGHTS bullet verbatim from origin/feat/issue-128-persist-insight:

#222 dead-ends @widest :   7123
#221 diversify @widest :   1194
#219 INSIGHTS bullet   :    463
#140 bar               :   1320
SUM of the four        :  10100  (tail budget 17940)

base= 20000 -> out= 31065 <=cap=True |#140 whole=True |#222 whole=True |#221 whole=True |#219 whole=True |notice=False
base= 45000 -> out= 56065 <=cap=True |#140 whole=True |#222 whole=True |#221 whole=True |#219 whole=True |notice=False
base= 59000 -> out= 59906 <=cap=True |#140 whole=True |#222 whole=True |#221 whole=True |#219 whole=True |notice= True
base= 60000 -> out= 59906 <=cap=True |#140 whole=True |#222 whole=True |#221 whole=True |#219 whole=True |notice= True

All four survive whole even when the head is over-cap and the elision fires — 10,100 of 17,940 tail chars, 7,840 to spare. Nothing is cut mid-structure and nothing is silently dropped (the notice is always present when elision happens). Note the tail is now 56% consumed at the widest; a fifth block over ~7.8 KB would start eating #219's bullet, which sorts first in the tail. Worth a comment for whoever adds the next one, not a change here.

Double-cap check. Feeding an already-capped 59,907-char render into _augment_instructions produces one notice, not two: after augment: 59905, notices: 1, #140 whole: True. And the single-cap invariant is pinned by test_the_bar_is_capped_by_the_shared_cap_not_a_second_one (line 378), which asserts src.count("cap_instructions") == 1 over inspect.getsource. Verified there is no second cap: harness.py:1092 is the only _oc.cap_instructions in _augment_instructions, and the sibling branches add none (#219 returns unwrapped; #221 routes through render_instructions(extra=...)).

Composed-tree caveat: git merge origin/feat/issue-128-persist-insight into #246 conflicts in harness.py (3 hunks), dashboard.py and skillopt.py — see merge-order note. My all-four measurement therefore composes the sibling blocks at their real widths through #246's real _augment_instructions rather than through a merged tree. The arithmetic is unaffected (all four land in the same tail, and cap_instructions is a pure function of length), but the merged tree should be re-measured once the conflicts are resolved.


Injection safety

Eval-cache hash: unchanged. hash_candidate_dir (cache.py:73-76) skips any path with a guidance component via _IGNORE_DIRS ⊇ INJECTED_DIRS. Crucially this is a directory-part check (any(part in _IGNORE_DIRS for part in rel.parts)), not the root-anchored basename check that PR #211 fixed — so the depth of the injected subtree is irrelevant. Verified empirically by test_the_reasoning_skill_is_not_mistaken_for_a_capability_edit (line 316), which copies the whole mechanism-probe tree into b/guidance/reasoning/ and asserts hash equality. That test is one of only 4 that pass on the base — i.e. it pins a pre-existing invariant rather than new behaviour, which is exactly right for a regression guard.

GEPA component list: unchanged. _NON_COMPONENT_DIRS (gepa.py:83) is likewise {".git","__pycache__"} | set(oc.INJECTED_DIRS), checked per path part. Same test asserts _components(a) == _components(b). So an injected read-context file cannot become an editable component and the optimizer cannot rewrite its own instructions. Both sets derive from the one INJECTED_DIRS tuple (optimizer_context.py:62), so no new enumeration was added.

Snapshot: excluded. "guidance" in _SNAPSHOT_IGNORE asserted at line 332; both run_step (harness.py:1546) and GEPA's accept branch pass it, so candidates stay capability-only.

Sealed-test leak: 0. Real zero-API runs, all three algorithms, greping every readable file in each workdir:

### hill-climb  sealed test ids ['a8','a7'] in workdir files: 0 []
### gepa        sealed test ids ['a8','a7'] in workdir files: 0 []
### skillopt    sealed test ids ['a8','a7'] in workdir files: 0 []

Cleaner than the PR's own library-level run, which reported 1 hits [('RUNMAP.md','a7')] — that is a substring collision (a candidate id containing a7), not a leak, and the shipped test (line 393) uses the same substring method and finds none. Either way 0.

Tamper guard (#197/#142): cannot false-fire. protect.project_dir_for scopes to .capevolve/project, resolve_protected takes exclude=<run dir> and returns early for anything under it, and _DEFAULT_GLOBS is ("adapters", "capevolve.yaml", *gold*). The injected subtree lives at run_dir/work/<cid>/guidance/reasoning/ — disjoint from the protected set by construction, and inside the excluded run dir besides.

diff -r rc=0 for all three, reproduced:

### hill-climb  workdir=cand_0001   diff -r vs source: rc=0 (identical)
    prompt 22164 / cap 60000 (36.9%)   bar present: True; honest wording: True
### gepa        workdir=gepa_0001   diff -r vs source: rc=0 (identical)
    prompt 23171 / cap 60000 (38.6%)   bar present: True; honest wording: True
### skillopt    workdir=so_e01s01   diff -r vs source: rc=0 (identical)
    prompt 22692 / cap 60000 (37.8%)   bar present: True; honest wording: True

Native placement verified separately — the mock optimizer has no skills_dir row, so my run-based probe only saw the guidance/ copy. Driving _inject_native_skills with claude-code confirms the second placement, and that the native copy correctly excludes scripts/ (the shared ignore at harness.py:1339):

 DIR .claude/skills/diagnose
 DIR .claude/skills/mechanism-probe
 DIR .claude/skills/tools
files under mechanism-probe: SKILL.md, meta.yaml

The glob("*/SKILL.md") loop (harness.py:1365) means a second reasoning skill needs no wiring, matching the claim.


Is a new component type warranted?

Marginally — I'd keep it, but the justification in the code is stronger than the justification in the PR.

For it: reasoning is genuinely a different kind of thing. Every other component is either sequenced by the orchestrate DAG (phase), selected as the optimization target (capability), chosen as the loop (algorithm), resolved as the proposer (optimizer), or the driver itself. mechanism-probe is none of those: it is injected read-context at one step, has provides: [], and is never invoked as a subprocess by the framework. Filing it under phase would make it eligible for DAG sequencing it must never enter; filing it under capability would make it look like an optimization target. The comment at build_manifest.py:25-29 says exactly this, and it's correct.

The cost is genuinely one line — COMPONENTS gained one string. build_manifest.py handles it with no other change (it reads component from meta.yaml and groups); the count is right; #213's _manifest_drift uses a */*/SKILL.md glob and a path-set comparison, so it picks the new tree up with no bump; nothing in core/ iterates the manifest by component (cli.py:136 resolves by name only).

Against it: one member, and the PR's own admission that it skipped first-principles because "nothing would load it" concedes the plural is aspirational. A component type whose population is 1 and whose second member has no loader is a taxonomy built for a future that may not arrive. But the alternative — mislabeling it as a phase — is worse than a slightly-early category, and unwinding it later is a one-line revert plus a manifest rebuild. Keep it.

The issue asked for first-principles too. Skipping it is the right call for the reason given, and the PR is honest about it. Do not add it until a loader exists.


Merge-order note

Recommended: #199#213#222#246#219#221.

Note the PR body says its base is 214 tests; the actual tip of origin/feat/issue-129-failure-memory (6d6cc52d) is 221 — the 214 figure predates #222's own review fixes. 221 + 19 = 240, so the arithmetic is consistent; the stated base number is just stale.


Verification I re-ran

Full suite on the branch (240, as claimed):

$ cd /tmp/rv-246 && PYTHONPATH=/tmp/rv-246/core python -m pytest core/tests -q
240 passed in 102.42s (0:01:42)

Full suite on the true base #222 (6d6cc52d) — 221, not the claimed 214:

$ cd /tmp/rv-222 && git log --oneline -1
6d6cc52d fix(algorithm): make the rejected-approach signature reflect the real edit (#129 review)
$ PYTHONPATH=/tmp/rv-222/core python -m pytest core/tests -q
221 passed in 90.37s (0:01:30)

(da9ad44d, the commit before that review fix, is 213. The PR's 214 matches neither; harmless.)

Fail-before, implementation stashed (git checkout 6d6cc52d + test file restored) — 15 failed / 4 passed, matching the claim exactly:

E  ImportError: cannot import name 'proposal_quality' from 'cap_evolve'
15 failed, 4 passed in 12.03s

The 4 that pass without the fix are regression guards on pre-existing invariants, correctly so:

test_a_genuine_mechanism_proposal_is_not_rejected PASSED
test_the_reasoning_skill_is_not_mistaken_for_a_capability_edit PASSED
test_the_bar_is_capped_by_the_shared_cap_not_a_second_one PASSED
test_no_test_split_id_reaches_any_injected_workdir_file PASSED

(Caveat: skills/reasoning/ survives a git checkout to the base only as untracked __pycache__, so the injection tests fail on ImportError rather than on a missing tree — the fail-before is real either way.)

compileall:

$ python -m compileall -q core/cap_evolve skills   →  rc=0

build_manifest.py — 21 skills, and the committed manifest is byte-identical:

wrote /private/tmp/rv-246/skills/_registry/manifest.json (21 skill(s))
  algorithm: agent-optimize, evograph, gepa, hill-climb, skillopt
  capability: mcp-tool, skill-package, system-prompt, tools
  optimizer: run-optimizer
  orchestrate: orchestrate, using-cap-evolve
  phase: baseline, diagnose, evaluate, finalize, gate, implement-and-check, intake, report
  reasoning: mechanism-probe
$ git status --short   →  (empty)

#213's lint on the branch — the new skill has zero errors AND zero advisories:

skill authoring lint — 21 skill package(s) under /private/tmp/rv-246/skills
  ERROR   capabilities/tools: SKILL.md body is 673 lines (>500); ...
  ERROR   capabilities/tools: SKILL.md body is ~10934 tokens (>5000); ...
  ERROR   orchestrate/using-cap-evolve: description must not contain XML tags
  advise  algorithms/evograph … phases/report          (11 advisories, none on reasoning/)
FAIL — 3 authoring violation(s) in 2 skill(s)

The 3 errors are genuinely pre-existing and genuinely #213's:

$ cd /tmp/rv-222   (base, 20 skills)
  ERROR   capabilities/tools: SKILL.md body is 673 lines (>500); ...
  ERROR   capabilities/tools: SKILL.md body is ~10934 tokens (>5000); ...
  ERROR   orchestrate/using-cap-evolve: description must not contain XML tags
FAIL — 3 authoring violation(s) in 2 skill(s)

$ cd /tmp/rv-213   (#213's own branch)
OK — 20 skill packages pass the authoring bar (0 advisory note(s))

Identical 3 on the base, and #213 clears all 3 plus every advisory. Nothing introduced here.

All 21 check.py print ok: true (run from each script's own dir; the new one in full):

{
  "skill": "mechanism-probe",
  "ok": true,
  "problems": [],
  "notes": [
    "run entry exposes main()",
    "all three declared fields parse from PROCESS.md",
    "false-rejection probe: a real mechanism proposal is never rejected",
    "declaration is recorded per candidate as advisory",
    "unfilled placeholders count as missing (no vacuous pass)"
  ]
}

Zero LLM calls — add-an-import probe (would a regression be caught?):

$ # added `import requests` to proposal_quality.py
$ python -m pytest core/tests/test_proposal_quality.py -k no_model -q
FAILED test_the_gate_makes_no_model_call
1 failed, 18 deselected

The guard is real. The module imports only re and pathlib; the banned list covers anthropic, openai, requests, urllib.request, http.client, aux_model, subprocess.

Parser edge cases (probed, informational):

multi-line value (continuation dropped)              declared= True   (first line only)
value is a bare code span `x`                        declared= True
one-char values                                      declared= True
'Observable:' without 'Expected'                     declared=False missing=['observable']
value >MAX_VALUE_CHARS                               truncated to 400
seed placeholder ABOVE an appended filled block      declared=False   <- non-blocking #2

Review of #246 (APPROVE WITH NITS) found the declaration parser inverted on the
most likely agent behaviour, a documented self-check that could not run, and a
false-rejection probe that only covered hill-climb.

1. `_field` used `re.search`, which takes the FIRST match — so an agent that
   appends its filled declaration BELOW the seed's `<...>` placeholders recorded
   as fully UNDECLARED. Advisory-only, so no run outcome changed, but it inverted
   the exact signal this feature exists to observe, on a very common shape. Now
   `finditer` scans every occurrence and skips placeholders, so the filled
   declaration wins wherever it sits. (Second first-match-only defect in this
   epic, after #189's guard.)

2. The `scripts/run.py` self-check documented in SKILL.md could not run: not from
   the optimizer's workdir (no `skills/` tree) and not from the injected copy
   (the bootstrap's upward walk never finds `core/`). `scripts/` was kept in the
   injected copy only to serve that dead command, so both are gone — one word in
   the existing `ignore_patterns`, matching the capability/diagnose copies. Also
   shrinks the injected read-context now four blocks share the prompt budget.

3. The advisory guarantee was pinned only on `run_step`. An enforcement injected
   into GEPA's LOCAL gate passed all 19 tests and surfaced only as five confusing
   test_gepa.py failures. Added per-algorithm probes for GEPA (on its own
   `gepa_local_gate` event) and SkillOpt, so the guarantee is pinned per
   algorithm rather than per code path.

Nits: `_PLACEHOLDER_RE`'s empty case is now its own alternative instead of a `*`
quantifier that happened to also match ""; a bare `Observable:` now parses as the
same field as `Expected observable:` (requiring the adjective recorded real
declarations as missing). The one-character-value floor is left as-is — the
declaration is presence-only by design, and any length bar would be arbitrary.

Honesty: the "historically wasted the iteration" claim is reworded in both
SKILL.md and PROMPT_BLOCK as the unvalidated hypothesis it is — nothing in this
repo measures knob-versus-mechanism edit outcomes, and the `proposal_quality`
event this PR adds is the instrument that would test it (zero rows so far).

244 tests pass (base #222 is 221, not the 214 the PR body stated; 221 + 23 = 244).
Copilot AI review requested due to automatic review settings July 30, 2026 22:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@OsherElhadad

Copy link
Copy Markdown
Collaborator Author

🔧 Review fixes

Commit 86514a79 on feat/issue-140-reasoning-skills. Verdict was APPROVE WITH NITS (0 blocking); all 3 non-blocking findings fixed, all 3 nits addressed, both claim corrections made.


The placeholder inversion — before/after

The shape: seed block untouched, filled declaration appended below it.

## Proposal declaration
- Mechanism: <what now behaves differently, and why>
- Hypothesis: <which cluster this fixes>
- Expected observable: <what will look different next iteration>

### My declaration
- Mechanism: quote_price recomputes the total in-body and refuses a mismatch...
- Hypothesis: the four tax-line failures share one root cause...
- Expected observable: get_total precedes every quote_price call...

BEFORE (2cb4d1dc, re.search — first match wins):

declared = False | missing = ['hypothesis', 'mechanism', 'observable']
mechanism = ''

AFTER (86514a79, re.finditer — placeholders skipped):

declared = True | missing = []
mechanism = 'quote_price now recomputes the total in-body and refuses a m'

Pinned by test_a_declaration_appended_below_the_seed_placeholders_still_counts. On the reviewer's point about finditer as the default: yes — this is the second first-match-only defect in the batch after #189's guard, and the whole parser is now occurrence-scanning rather than first-hit, so a future field added to _LABELS inherits the correct behaviour instead of re-introducing the bug.


The GEPA-local-gate enforcement probe

Injected a real enforcement at gepa.py:643, honest wording untouched:

        local_pass = _sum_reward(child_mb) > _sum_reward(parent_mb)
        if not _q140["declared"]:
            local_pass = False  # INJECTED ENFORCEMENT (probe)

The new probe catches it directly, and names the algorithm:

FAILED core/tests/test_proposal_quality.py::test_gepas_local_gate_is_not_enforcing_the_declaration
E  AssertionError: an UNDECLARED candidate was stopped at GEPA's LOCAL gate — the advisory
   guarantee is broken for gepa: {'kind': 'gepa_local_gate', 'candidate': 'gepa_0001',
   'parent': 'seed', 'child_sum': 3.0, 'parent_sum': 0.0, 'passed': False}
1 failed, 22 passed

Enforcement removed; git diff --stat core/cap_evolve/gepa.py is empty and the file is back to 23/23 green. Before this, that injection passed all 19 tests and surfaced only as five confusing test_gepa.py failures.


Numbered response to all six findings

1. FINDING 1 — dead self-check, and scripts/ copied only to serve it. Fixed. Both halves gone. scripts added to the existing ignore_patterns at harness.py:1281 (one word, now identical to the capability/diagnose copies) and the ## Checking your own declaration bash block replaced with a re-read instruction that needs no runnable command. The reviewer is right that it could not work either way: not from the optimizer's workdir (no skills/ tree) and not from the injected copy (the bootstrap's upward walk never finds core/). The stale comment claiming scripts/ was deliberately kept is replaced with the real reason it is excluded. Injected copy is now 2 files instead of 5 — a small read-context saving that matters more now four blocks share the tail.

2. FINDING 2 — the signal inverts on append-below. Fixed. Evidence above. re.finditer + placeholder skip, so the last/filled declaration wins over an unfilled one wherever it sits.

3. FINDING 3 — the false-rejection probe was hill-climb only. Fixed. Two per-algorithm probes added:

4. Nit — _PLACEHOLDER_RE's [-–—.]* matches "". Fixed. The empty case is now its own alternative (^\s*$|^\s*(?:<.*>|[-–—.]+|...)\s*$) and the dash branch requires at least one dash, so the pattern reads as what it rejects.

5. Nit — expected\s+observable required the adjective. Fixed. Now (?:expected\s+)?observable, so a bare Observable: is the same field. The reviewer flagged this as brittleness the docstring claimed tolerance for; the docstring is now true. New test: test_a_bare_observable_label_is_the_same_field.

6. Nit — one-character values count as declared. Declining, deliberately. The declaration is presence-only by design, exactly because the judgement half is not mechanically decidable. Any minimum length would be an arbitrary number doing a knob's job — a 20-char bar rejects a terse-but-real "in-body guard in quote_price" while passing 20 chars of filler. The floor is documented in MAX_VALUE_CHARS's neighbourhood and the reviewer's own framing ("worth knowing the signal's floor") is the right disposition. If the recorded rows ever show one-char values being used to game a bar, that bar does not exist yet to game.


Claim correction 1 — the base number

The PR body's 214 is wrong. The true tip of #222 (6d6cc52d) is 221; 214 predates #222's own review fix (da9ad44d is 213, matching neither). So the arithmetic is: 221 base + 19 original = 240, and 221 + 23 = 244 with the 4 tests added here. Restated.

Claim correction 2 — the rationale is a hypothesis, and now reads as one

The reviewer is right that "an edit you cannot state a mechanism for is the edit that historically wasted the iteration" cited "any run's LEDGER.md" with no run, no number and no artifact, and that nothing in-repo measures knob-versus-mechanism outcomes. Given this epic's history with unsubstantiated claims (#99's headline, #241's 8 removals, #244's "check does not run run_target"), the wording mattered. SKILL.md now says, verbatim:

This is an unvalidated hypothesis, stated as one: we believe an edit whose mechanism cannot be stated is the edit that wastes the iteration, but nothing in this repo has measured knob-shaped versus mechanism-shaped edit outcomes — the proposal_quality event this skill produces is the instrument that would test it, and it currently has zero rows.

PROMPT_BLOCK carries the same correction in one clause: "our working hypothesis (unmeasured so far; this record is how we would test it)". The honest-wording tests still pin the enforcement claim; a reword to enforcing language still fails 4 tests.


Verification

$ PYTHONPATH=/tmp/fx-246/core python -m pytest core/tests -q
244 passed in 120.00s (0:01:59)

244 = 221 (true #222 base) + 19 (original) + 4 (new: append-below, bare-Observable, GEPA local gate, SkillOpt step). 0 failed.

Injected copy — scripts/ out, diff -r still rc=0 for all three, sealed-test leak 0:

hill-climb  scripts/ present: False  files: ['SKILL.md', 'meta.yaml']
            diff -r (-x scripts) rc=0 (identical)
            prompt 22240/60000  bar whole: True  no run.py cmd in SKILL: True
            sealed test ids ['a8', 'a7'] in workdir files: 0 []
gepa        scripts/ present: False  files: ['SKILL.md', 'meta.yaml']
            diff -r (-x scripts) rc=0 (identical)
            prompt 23247/60000  bar whole: True  no run.py cmd in SKILL: True
            sealed test ids ['a8', 'a7'] in workdir files: 0 []
skillopt    scripts/ present: False  files: ['SKILL.md', 'meta.yaml']
            diff -r (-x scripts) rc=0 (identical)
            prompt 22768/60000  bar whole: True  no run.py cmd in SKILL: True
            sealed test ids ['a8', 'a7'] in workdir files: 0 []

Nothing else depended on scripts/: the two remaining files are byte-identical to source, and the only reference to run.py anywhere was the SKILL.md line now deleted. skills/reasoning/mechanism-probe/scripts/ stays in the repo — check.py runs there, from a checkout where the bootstrap does find core/, and it still prints ok: true.

Cache hash + GEPA component list unchanged:

hash equal: True | components equal: True | guidance in _SNAPSHOT_IGNORE: True

Composed prompt capped, bar whole, notice present when elision fires:

  in=  20000 out= 22360 <=cap=True bar whole=True notice=False
  in=  45000 out= 47360 <=cap=True bar whole=True notice=False
  in=  59000 out= 59905 <=cap=True bar whole=True notice=True
  in=  60000 out= 59905 <=cap=True bar whole=True notice=True
  in= 110000 out= 59906 <=cap=True bar whole=True notice=True
bar size: 1396 chars

The bar grew 1,320 → 1,396 chars from the honesty rewording (+76, still ~2.3% of the cap and well inside the tail's 17,940). The four-blocks-at-widest measurement is unaffected: 10,100 → 10,176 of 17,940, 7,764 to spare.

compileall, manifest, checks, lint:

$ python -m compileall -q core/cap_evolve skills   →  rc=0

$ python skills/_registry/build_manifest.py
wrote /private/tmp/fx-246/skills/_registry/manifest.json (21 skill(s))
  reasoning: mechanism-probe
$ git status --short   →  manifest.json unchanged (byte-identical)

all 21 check.py:  ok:true = 21   not-ok = 0

#213's lint on this branch:
  ERROR   capabilities/tools: SKILL.md body is 673 lines (>500)
  ERROR   capabilities/tools: SKILL.md body is ~10934 tokens (>5000)
  ERROR   orchestrate/using-cap-evolve: description must not contain XML tags
FAIL — 3 authoring violation(s) in 2 skill(s)

Same 3 pre-existing errors the reviewer confirmed on the base and on #213's own branch — zero errors and zero advisories on reasoning/.


Merge order

#199#213#222#246#219#221.


Files touched

  • core/cap_evolve/proposal_quality.pyfinditer + placeholder skip, bare Observable:, _PLACEHOLDER_RE empty alternative, PROMPT_BLOCK honesty clause
  • core/cap_evolve/harness.pyscripts added to the reasoning-copy ignore_patterns; stale comment corrected
  • core/tests/test_proposal_quality.py — 4 new tests (19 → 23)
  • skills/reasoning/mechanism-probe/SKILL.md — dead self-check command removed, rationale reworded as an explicit unvalidated hypothesis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

algorithm Optimization algorithms: GEPA / SkillOpt / hill-climb dashboard Dashboard backend/frontend enhancement New feature or request observability Live run visibility, logging, tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

On-demand reasoning skills + a "mechanism-not-knob" proposal quality gate

3 participants