feat(algorithm): on-demand reasoning skills + an advisory proposal-quality declaration (#140) - #246
feat(algorithm): on-demand reasoning skills + an advisory proposal-quality declaration (#140)#246OsherElhadad wants to merge 13 commits into
Conversation
…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.
|
🏷️ Automatic Labeling I've analyzed this pull request and added the following labels:
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. |
| 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 |
| "**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 |
🔬 EvidenceAll commands run in 1. Baseline on the #222 base, before my change2. Real end-to-end through the actual CLI — all three deterministic algorithmsEach is a genuine
3. The same three algorithms driven directly through the library loopsAdds the verbatim prompt text of the gate and the full 4. Composed-prompt measurement — #219 + #221 + #222 + #140, and the overflow probe#219's INSIGHTS pointer bullet is taken verbatim from 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 probeThe three that matter for the "risky half":
6. Fail-before / pass-afterTest file kept, implementation stashed ( (That run predates the dashboard test, hence 18 collected; with it, 19.) After 7. Full suite, including the #200-flaky dashboard test240 passed, 0 failed. 214 (base) + 19 (new) + 7 ( 8.
|
🔍 Review — PR #246Verdict: APPROVE WITH NITS. The central claim holds. I traced every consumer of the BlockingNone. Non-blocking1. The SKILL.md tells the optimizer: python skills/reasoning/mechanism-probe/scripts/run.py --process ./PROCESS.mdThe optimizer's cwd is the workdir, which has no And the path that does exist fails too, because (That import resolved to my editable install of 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 Fix (pick one): either change line 115 to the workdir-relative path ( 2.
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 ( 3. 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 ENFORCEMENTAll 19 of #140's tests still pass: It is caught, but by pre-existing tests in a different file ( 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 Nits
Is "advisory" true in the code?Yes. Every path traced: Producers. The only two call sites of Acceptance sites, each checked:
Event-consumer surface. Wording-pinning test — does it fail on a reword? Yes, and it fails on the substantive claim, not a stray phrase. I rewrote Four tests, not one — and three of them fail because Better still, the behavioural half is guarded too. I injected a real enforcement into accepted = decision.accept
if not _q["declared"]:
accepted = False
FAILED test_an_undeclared_proposal_is_also_not_rejected
AssertionError: an UNDECLARED proposal was rejected — the gate is not advisorySo 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 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 Composed prompt at the boundaryPR's overflow probe, reproduced exactly: 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 No overshoot at any input size, unlike #219's original 98-char miss. The arithmetic is sound because All four blocks at their bounded widest, simultaneously. I saturated 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 Composed-tree caveat: Injection safetyEval-cache hash: unchanged. GEPA component list: unchanged. Snapshot: excluded. Sealed-test leak: 0. Real zero-API runs, all three algorithms, greping every readable file in each workdir: Cleaner than the PR's own library-level run, which reported Tamper guard (#197/#142): cannot false-fire.
Native placement verified separately — the The 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: The cost is genuinely one line — Against it: one member, and the PR's own admission that it skipped The issue asked for Merge-order noteRecommended: #199 → #213 → #222 → #246 → #219 → #221.
Note the PR body says its base is 214 tests; the actual tip of Verification I re-ranFull suite on the branch (240, as claimed): Full suite on the true base #222 ( ( Fail-before, implementation stashed ( The 4 that pass without the fix are regression guards on pre-existing invariants, correctly so: (Caveat: compileall: build_manifest.py — 21 skills, and the committed manifest is byte-identical: #213's lint on the branch — the new skill has zero errors AND zero advisories: The 3 errors are genuinely pre-existing and genuinely #213's: Identical 3 on the base, and #213 clears all 3 plus every advisory. Nothing introduced here. All 21 Zero LLM calls — add-an-import probe (would a regression be caught?): The guard is real. The module imports only Parser edge cases (probed, informational): |
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).
🔧 Review fixesCommit The placeholder inversion — before/afterThe shape: seed block untouched, filled declaration appended below it. BEFORE ( AFTER ( Pinned by The GEPA-local-gate enforcement probeInjected a real enforcement at 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: Enforcement removed; Numbered response to all six findings1. FINDING 1 — dead self-check, and 2. FINDING 2 — the signal inverts on append-below. Fixed. Evidence above. 3. FINDING 3 — the false-rejection probe was hill-climb only. Fixed. Two per-algorithm probes added:
4. Nit — 5. Nit — 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 Claim correction 1 — the base numberThe PR body's 214 is wrong. The true tip of #222 ( Claim correction 2 — the rationale is a hypothesis, and now reads as oneThe reviewer is right that "an edit you cannot state a mechanism for is the edit that historically wasted the iteration" cited "any run's
Verification244 = 221 (true #222 base) + 19 (original) + 4 (new: append-below, bare-Observable, GEPA local gate, SkillOpt step). 0 failed. Injected copy — Nothing else depended on Cache hash + GEPA component list unchanged: Composed prompt capped, bar whole, notice present when elision fires: 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: Same 3 pre-existing errors the reviewer confirmed on the base and on #213's own branch — zero errors and zero advisories on Merge order#199 → #213 → #222 → #246 → #219 → #221.
Files touched
|
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 "✅ onecopytree" assessment held: the file half of this is literally onecopytreeon #199'sinjectseam.What the skills are and when they load
A new
reasoningskill 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-principlesskill. 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:
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.mdanddocs/ARCHITECTURE.mdsay "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 thePROCESS.mdthe optimizer already writes.cap_evolve.proposal_qualityparses those and logs aproposal_qualityevent 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 ofmechanism/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 owncheck.pycarries 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_callpins it (noanthropic/openai/requests/urllib/aux_model/subprocessin the module). Noaux_modeltier 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_oneasserts_augment_instructionsappliescap_instructionsexactly once.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_wholecrosses the bound (the #219 lesson — a pinning test whose fixture never overflows proves nothing): it feeds 110,000 chars through the real_augment_instructionsand 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 runonexamples/toy_calcwith themockoptimizer, zero API cost, all three deterministic algorithms.diff -ragainst source is rc=0 for every one:diff -rvs sourceproposal_qualityloggedcand_0001gepa_0001so_e01s01No-leak proof
grepfor 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_filepins 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'sINJECTED_DIRS, andtest_the_reasoning_skill_is_not_mistaken_for_a_capability_editproves the hash and component list are unchanged by its presence.Expected merge order
#199→#212→#211→#219/#221/ #222 → this. This branch is cut fromorigin/feat/issue-129-failure-memorybecause it must route through #222'scap_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
214 on the #222 base + 19 new + 7
test_dashboard_launch(which passed here). 0 failed.Fail-before / pass-after — test file present, implementation stashed:
then restored:
19 passed.