feat(algorithm): re-inject rejected approaches as optimizer constraints (#129) - #222
feat(algorithm): re-inject rejected approaches as optimizer constraints (#129)#222OsherElhadad wants to merge 11 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
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| 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.", |
|
❌ Automatic Labeling Failed An error occurred while trying to automatically label this pull request. Please check the workflow logs for details and add labels manually. |
🔬 EvidenceAll commands run in 1. Merge base2. Fail-before (stash the implementation, keep only the new test file)3. Pass-after + merged-tree baseline4. Only-new-tests differ from the merged baseline5. Real end-to-end runs (
|
🔍 Review — PR #222Verdict: CHANGES REQUESTED The honesty correction is right and the split it names (advisory at the prompt / hard at the gate) is the correct framing — that half I'd merge today. But the injected content is wrong on the paths that matter most: for every real agent optimizer ( BlockingB1 — Real And the collapse, with a realistic snapshot-vs-workdir pair and two different prompt edits: Consequence: on every non-mock optimizer the memory is not just useless, it is actively misdirecting — it dedupes unrelated approaches into one row and tells the optimizer it re-proposed something it never proposed. This also poisons _CAP_DIFF_SKIP = set(_oc.SCRATCH_NAMES) | set(_oc.INJECTED_NAMES)
_CAP_DIFF_SKIP_DIRS = set(_oc.INJECTED_DIRS)and in B2 — signature truncation is head-first, so any edit to a large capability yields a signature that does not contain the edit. Consequence: the dedupe key is not a function of the edit for realistic capabilities (a SKILL.md, a system prompt of any length). Same false-positive as B1. body = " | ".join(parts)
if len(body) <= max_chars:
return body
digest = hashlib.sha256(body.encode()).hexdigest()[:12]
return body[:max_chars - 20].rsplit(" | ", 1)[0] + f" … [{digest}]"B3 — eviction is by first appearance, not recency, so a just-re-proposed dead end can be the one that gets evicted. Consequence: the single most predictive row — the approach the optimizer just re-emitted — is the one dropped, on exactly the runs (>12 distinct dead ends) where the block matters. Also silently invalidates the "12 most recent" wording the block prints to the optimizer. seen[approach]["count"] += 1
seen[approach] = seen.pop(approach) # re-proposed => most recent
continue
B4 — the "one definition, four consumers" claim is false: a fifth copy is untouched, and if rel.name in ("INSTRUCTIONS.md", "MEMORY.md", "STATE.md",
"LEDGER.md", "JOURNAL.md", "PROCESS.md", "RUNMAP.md"):Consequence: SkillOpt's applied-edit-budget count still inflates by GEPA-style scratch when present, and the header comment now misdescribes the code — which is the failure mode this whole change exists to correct. Non-blockingN1 — Not this PR's job, but the comment reads as if the drift is fully resolved. See the SCRATCH_NAMES section. N2 — N3 — control characters survive into Not exploitable today on this branch: N4 — no frontend test for the new N5 — Nits
Is the new honesty wording precise?Old (
New (
Verdict: precise, and the strongest part of this PR. It makes exactly the claim the system can support and no more. Specifically:
Two things stop it from being airtight, both mechanical rather than rhetorical:
The four claims #212 added are all corrected, and correctly. A scan of the merged baseline ( And crucially the new Signature correctnessStable: yes. Distinct: no. The PR's stability claim holds; the distinctness property it silently depends on does not. Stability (all pass): Distinctness fails two independent ways — B2 (300-char head truncation, Bound holds comfortably, and beats the author's claim: Repeat count and gate reason are accurate against the run record. GEPA e2e: 6 All five rejection sites record No missed site. The merge gates correctly diff against the common ancestor ( Three competing SCRATCH_NAMES unificationsThey do not conflict semantically but they do conflict textually, and #222's is the weakest of the three.
#211 is correct and is the one to keep. It is at the bottom of the import graph ( #222 does NOT reintroduce #211's data-loss hazard. Verified directly:
#211's own invariant tests already pass against #222's sets (they assert superset, not equality): Single recommended end state:
That's one definition per operation, five consumers, one destructive filter, no third symbol. Composed prompt with #219I built the composed tree by hand (
Capped: yes, comfortably. With both blocks live, real e2e prompts are nowhere near the ceiling and Truncation behaviour when the base prompt alone overflows — the constraint block sits in the tail slice, so it survives whole rather than being half-cut: The 70/30 head/tail split in Coherent: mostly, with one genuine redundancy. The two blocks read as complementary rather than duplicative — #219's The gate reason string is verbatim-duplicated per row, and #219 lists each repeat separately where #222 correctly collapses them — so the optimizer sees "5 separate things hurt" next to "1 distinct approach, re-proposed 5x". Whichever merges second should add a one-line pointer ( And the composed tree reproduces B1 in a second form, which is independent evidence that the skip list is the root cause: The composed e2e block shows the same thing reaching the prompt verbatim. Fixing B1 properly (derive from one union, add each new framework-written file there) makes this a non-event; resolving the conflict by adding Merge-order note
#204/#218 are frontend-only and don't touch Verification I re-ranSuite, branch — matches the claimed 213: Merged-tree baseline at
Fail-before, by reverting only the implementation files to the merge base and keeping the new test file: 13/13 reproduced. (Note Orchestration-mode flake claim: VERIFIED, and the diagnosis is exactly right. It is an editable-install artifact, present on both sides, and it disappears with Not random-ordering-dependent ( E2E, real Baseline reproduction of the #1 claim — the same run on Both halves of the old GEPA capability-diff bug: real, and fixed for the read-side filters. On the baseline, a GEPA candidate's capability diff leads with its own reflective scratch: On the branch all four read-side filters agree and the GEPA e2e signature is clean ( Frontend — No new frontend test for the Leak scan — no split id reaches the block;
Commit authorship is correct: |
… 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).
Review fixes for PR #221. Two of the four blocking findings made the feature kill or degrade productive runs, which is the failure direction this module exists to prevent. 1. SkillOpt iterations were counted TWICE. `harness.run_step` logs `step` for a candidate and `skillopt.py` then logs `skillopt_step` for the SAME candidate; both were in the iteration-kind list, so a window=6 ladder reached `stop` at iteration 5 of a productive run, and a genuine near-miss was recorded once alive and once dead (the duplicate carries no parent_val). Dropped `skillopt_step` from the pre-#199 `_FALLBACK_KINDS` literal. PR #219 owns the root fix in `rundir.ITERATION_EVENT_KINDS`; no second dedup layer here, and deliberately NOT id-keyed dedup, which drops resumed SkillOpt iterations. 2. `NameError: cannot access local variable 'why'` in `hill_climb_loop` when the loop body never runs (max_iterations=0, or a resume with no budget left). `why` is now initialised next to `steps`. 3. The plateau block escaped the prompt cap and silently lost data. It now travels as `extra=` through `_augment_instructions` / `run_step`, so it is inside #222's MAX_INSTRUCTIONS_CHARS (measured 64941 -> 2625) and lands in the TAIL that truncation preserves — it is the only behavioural block of the three, so it must not be the first casualty. Removed the bare rejected-ids line entirely: it called `RejectedMemory.entries()` (removed by #199) behind a bare `except` that swallowed the AttributeError, and duplicated #222's richer signature channel. `prompt_block` no longer accepts `rejected=`, so a caller passing one now fails loudly. Bounded `exhausted_lineages` to 6 ids, 60 chars each. 4. GEPA was steered away from its BEST lineage. `exhausted_lineages` reused `_dead()`, which counts accepted-but-not-global-best as dead, so a lineage whose children were ALL accepted was dropped from the Pareto sampling pool — with no ratchet on that path and no stop event. Per-lineage exhaustion now uses its own narrower `_lineage_dead()`: an accept is never dead ground for a lineage, because widening the per-instance frontier is GEPA's mechanism. The accepted-but-not-best clause stays at the GLOBAL level, where the ratchet makes it safe. Also, from the non-blocking findings: - The ladder was unreachable at the shipped defaults: `stall: 2` stopped every run at 3 iterations, before `plateau_window: 6`. `stall` now defaults to 0 (off) with the reasoning in the template, so the shipped product actually runs the ladder. All evidence is re-run at the shipped default. - The ratchet now caps escalation at `warn` rather than `diversify`. A run still clearing the honest val gate every iteration gets a warning and no behavioural intervention, so the diversify block can never tell an optimizer a lineage failed when it was accepting. - The reason string no longer claims "no near-miss in that streak" when the streak is all accepts. - `series` carries the real signed delta for `gepa_local_gate` rows instead of hardcoding 0.0, so a tie is distinguishable from a regression. Deadness is unchanged (gepa's pass condition is a strict `>`). - An explicit `plateau_window: 0` now means off instead of silently reverting to 6. - Plateau state reaches the React dashboard: `plateau_level` on the hub row, `plateau` / `exhausted_lineages` typed on the detail summary, and a KPI tile. It is a separate field from `status` on purpose — that one is liveness, this one is progress. - Documented the honest cost of the delta<=0 rule (N regressions then a breakthrough is stopped at N) and that `--resume` deliberately carries the streak. Tests: 215 core (was 207) + 44 dashboard backend. 8 new regression tests, one per blocking finding plus resume, the bounded block, and the local-gate delta. `test_plateau_block_reaches_the_prompt_inside_the_cap` asserts the block reaches the prompt, so the #199 prompt-assembly merge trap fails loudly instead of compiling clean with the feature dead.
🔧 Review fixesAll 4 blocking, 5 non-blocking and 3 nits addressed. Rebased onto #211 and its Branch: The verdict's core claim, confirmed and fixedThe reviewer was right that the block was actively misdirecting on every real optimizer, and right about the root cause. The asymmetry that makes B1 bite is worth stating plainly because it explains B1 and B4 at once: a capability diff compares a SNAPSHOT parent (already stripped by B1 evidence — real
|
| before | after | |
|---|---|---|
| sole definition | 3 competing symbols | rundir.SCRATCH_NAMES / LEGACY_SCRATCH_NAMES / NON_CAPABILITY_NAMES (#211) |
optimizer_context.SCRATCH_NAMES |
10-tuple, flat | deleted |
| read-side filters | 4, one pre-drift 5th | 5, all NON_CAPABILITY_NAMES | INJECTED_NAMES + INJECTED_DIRS |
| destructive filter | INJECTED_* + 3 names |
INJECTED_* + SCRATCH_NAMES (live writers only), root-anchored |
INJECTED_DIRS/NAMES home |
optimizer_context |
unchanged — inject() writes them, rundir stays unaware |
#211's own superset test is extended to pin the INJECTED_* half as well, since that is B1's root cause and a hardcoded subset is exactly how the previous four copies drifted:
for name, names, dirs in (("cache", …), ("gepa", …), ("skillopt", …),
("dashboard", …), ("harness_capdiff", …)):
assert set(INJECTED_NAMES) <= set(names)
assert set(INJECTED_DIRS) <= set(dirs)#219 must rebase on both #211 (the name sets moved to rundir; add INSIGHTS.md to the shared INJECTED_NAMES, not to a literal) and #212 (whose _augment_instructions signature is the right resolution — restoring the dropped rejected, history params is what produced the reviewer's 40 failed).
Coherence with #219 — proposed division of labour
Yours = what helped/hurt numerically. Mine = what was tried and rejected, and how often. Neither is derivable from the other, and the composed prompt confirms both fire without competing for the same budget. Concretely:
#219 INSIGHTS.md |
#222 constraint block | |
|---|---|---|
| axis | val Δ per candidate, which tasks broke, what's still open | the exact edit signature, the gate reason, the repeat count |
| unit | one row per rejection | one row per distinct approach |
| delivery | a file in the workdir + a pointer paragraph | appended text in the prompt tail |
| lives in | its own file (no prompt-budget competition) | the protected 30% tail slice |
Three asks, all yours to land since #219 merges last:
- Dedupe
INSIGHTS.md's HURT rows onapproach. The composed run shows the honest problem the reviewer named — 5 separate HURT rows next to my 1 row sayingre-proposed 5x (latest cand_0005):Same six events, one collapsed and one not. That reads as a contradiction to the optimizer.## What was REJECTED by the gate (largest movers first …) - iter 5 `cand_0005` val Δ +1.000 (Δ=+1.0000 <= 1000000000.0000) — while fixing {a1, a4} - iter 4 `cand_0004` val Δ +1.000 (Δ=+1.0000 <= 1000000000.0000) — while fixing {a1, a4} - iter 3 `cand_0003` … - iter 2 `cand_0002` … - iter 1 `cand_0001` … - Add the cross-pointer on the HURT heading: "see the ALREADY TRIED & REJECTED block for the exact edits" — and drop the verbatim gate-reason string per row, since my block already quotes it once per distinct approach.
- feat(algorithm): plateau/convergence detection with escalation + per-lineage exhaustion #221's bare rejected-id line should go in favour of this block, per the earlier suggestion.
Nothing to remove on my side: the duplication is #219's rows, not mine, and my rows are already the collapsed form.
Composed prompt, capped comfortably (#199+#212+#211+#222+#219, optimizer_name="claude-code", 6 iterations):
composed hill-climb, last iter: workdir=cand_0006 chars=22261 (cap 60000)
both blocks live? #222 block: True | #219 INSIGHTS pointer: True
INSIGHTS.md is a FILE not appended text: True | its size: 1803
Also documented in cap_instructions' docstring (nit 3): the kept tail is 30% of the budget (18 KB at the 60 KB ceiling), the appended blocks live in it, my block measures ~7 KB at 200 rejections — so it survives an overflow whole rather than cut mid-list. Anything appended after that point must stay well under 30%.
Numbered response to all 12 findings
| # | Finding | Response |
|---|---|---|
| B1 | _CAP_DIFF_SKIP omits INJECTED_*; hardcoded 3-of-9 dirs |
Fixed. Both sets derived, in all 5 consumers. New test runs run_step with optimizer_name="claude-code" and asserts the real edit is present and no injected file leaks. |
| B2 | head-first truncation → not injective | Fixed. sha256 of the whole normalized body on overflow. PROBE C now COLLAPSED? False. |
| B3 | eviction by first appearance | Fixed. A repeat requeues its row. Test with a repeat case (the 50-distinct bound test structurally cannot catch it). |
| B4 | 5th scratch copy in skillopt |
Fixed + given the INJECTED_* half too, since it also walks snapshot-vs-workdir. Covered by #211's superset test. |
| N1 | unification doesn't fix #110; _SNAPSHOT_IGNORE untouched |
Resolved by the #211 rebase, not by a comment change: _SNAPSHOT_IGNORE = INJECTED_DIRS + INJECTED_NAMES + SCRATCH_NAMES, so FOCUS.md/REFLECTION.md are now stripped and no LEGACY name reaches it (leaks any LEGACY name? NO above). The comment that "read as if the drift is fully resolved" is now accurate. |
| N2 | cid is the first but the count is the latest |
Fixed. Rows now read **cand_0001**, re-proposed 5x (latest cand_0005). Keeping the first cid is deliberate (its reason is what's quoted); the ambiguity is gone. |
| N3 | control chars survive | Fixed at harness.py:904 as suggested, and widened: your PROBE E showed the U+202E RTL override surviving my first pass, which is the actual spoofing vector. Stripped: C0 + DEL + bidi embedding (U+202A–U+202E) + bidi isolates (U+2066–U+2069). Emoji and non-latin text survive — pinned by a test. 'prompt.txt: +[31mRED RTL 😀' → ESC? False | BEL? False | any C0/DEL? False. |
| N4 | no frontend test for approaches |
Fixed. Two cases in the existing describe('normalizeReason + deadEnds'): dedupe + ≤3 cap, and [] on pre-#129 records. vitest 45 → 47 passed. |
| N5 | reads the capability twice per rejection | Noted, declining, with a ponytail: comment naming the ceiling and the upgrade path (accept the already-computed _diff_capabilities text as an optional arg). Negligible next to the rollouts that just ran, and free on an accept — threading a cache through 5 call sites buys nothing at current scale. |
| nit 1 | function-body cap_instructions import |
Fixed → _oc.cap_instructions(...). |
| nit 2 | -> block <~ 5 KB understates |
Fixed → <8 KB (your measurement: 7123 chars at 200 rejections). |
| nit 3 | cap_instructions docstring vague on the tail |
Fixed — 30% tail, the block lives in it, ~7 KB at 200 rejections, and the constraint on anything appended later. |
Reproduction recipe corrected, thank you — git stash push -- <impl> reports No local changes to save on a clean worktree, exactly as you found:
### PR's original recipe (git stash push) on a clean worktree ->
No local changes to save
The isolated per-fix reverts above are what I used instead; they're tighter than a whole-file git checkout <base> -- on this tree, which now also reverts the #211 rebase and produces 26 unrelated ImportError failures rather than a clean signal.
Verification
$ cd /tmp/fx-222 && /tmp/ce-venv/bin/python -m compileall -q core skills
compileall clean (exit 0)
$ PYTHONPATH=/tmp/fx-222/core /tmp/ce-venv/bin/python -m pytest core/tests -q
221 passed in 90.44s (0:01:30)
| tree | tests |
|---|---|
main + #199 + #212 (previous baseline) |
200 passed |
+ #211 (new rebase base, b8dbed3) |
204 passed |
+ #222 (this branch, 6d6cc52) |
221 passed (+17: 13 original + 4 new) |
+ #219 (composed, all five) |
234 passed, 0 failed |
The composed number is the one worth calling out: the reviewer measured 40 failed on the naive composition. Resolved in the order #199 → #212 → #211 → #222 → #219, taking #212's _augment_instructions signature and letting INSIGHTS.md arrive through the shared INJECTED_NAMES, it is 234 passed / 0 failed. INSIGHTS.md no longer becomes the signature — the failure you saw (signature does not name the edited file: INSIGHTS.md: +# INSIGHTS — durable priors…) was B1 in a second form, and fixing the union properly makes it a non-event, as you predicted.
Re-proved, unchanged:
5 rejection sites still record `approach`:
core/cap_evolve/harness.py:1518 run_step val gate approach_signature(parent_dir, workdir)
core/cap_evolve/gepa.py:655 GEPA local minibatch gate approach_signature(parent_dir, workdir)
core/cap_evolve/gepa.py:694 GEPA full-val gate approach_signature(parent_dir, workdir)
core/cap_evolve/gepa.py:826 GEPA merge local gate approach_signature(anc_dir, workdir)
core/cap_evolve/gepa.py:853 GEPA merge val gate approach_signature(anc_dir, workdir)
### bound — 200 rejections
block chars: 7123 | rows: 12 | <8000? True | under MAX? True | newest kept? True | oldest dropped? True
split ids (test+val) leaked into block: NONE
max INSTRUCTIONS.md: 21763 chars (cap 60000) # branch only
composed (#222 + #219): 22261 chars (cap 60000)
Frontend:
$ npx tsc -b → tsc rc=0
$ npm test → Test Files 13 passed (13) | Tests 47 passed (47) (was 45)
RUN.md — the two sentences are now true of the code
Wording unchanged (it was the right wording; the code just hadn't met it):
- "re-injected into every later proposal prompt … carrying the exact edit signature" — true now that B1/B2 are fixed. Before, a real agent optimizer's signature usually didn't contain the edit at all; PROBE B1/G above show it does, and PROBE C shows it's the exact one rather than a prefix shared with three other edits.
- "the repeat is counted in the constraint block" — true now that B3 is fixed. Before, a re-proposed approach could be evicted while newer one-offs were kept.
Files touched
core/cap_evolve/harness.py · dashboard.py · skillopt.py · optimizer_context.py · gepa.py · cache.py · rundir.py (via the #211 merge) · core/tests/test_failure_memory.py · core/tests/test_gepa.py · dashboard/frontend/src/test/insights.test.ts
Commits authored Osher Elhadad <Osher.Elhadad@ibm.com>, no Co-Authored-By.
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).
Closes #129
The honesty gap first: was "never re-proposed" true?
No — it was false on both halves.
RUN.mdclaimed:Before this PR:
rejected.jsonlwas written but its only reader was the dashboard. PR refactor(core): drop dead optimizer-memory API + unused params; fix misleading cache docstring #212 (issue Remove write-only optimizer-memory (memory.py) + unused params; fix misleading cache.py docstring #114) deleted the write-only memory whoserender()never reached a prompt and left an explicit note:_augment_instructionsis the only function whose output reaches the optimizer. Nothing routed rejections there.The e2e transcript below shows the failure concretely: with a mock optimizer proposing the same bad edit, hill-climb re-proposed the identical rejected edit 5 times and the prompt never mentioned it once. That's 4 wasted full-val evals per run.
RUN.mdanddocs/COMPARISON.mdare corrected to state what's actually true.What changed
harness.approach_signature(parent_dir, cand_dir)— a stable, compact signature of what an edit changed, from the capability diff (_diff_capabilities, the same source the dashboard/RUNMAP use, so it never picks up injected read-context or algorithm scratch): per touched file, whitespace-collapsed added/removed lines. Cosmetic variants of one idea collapse to one signature. A no-op edit (optimizer errored → workdir is a verbatim parent copy) yields""and is not injected.harness.dead_end_constraints(run_dir)— the## ALREADY TRIED & REJECTEDblock: deduped signature + gate reason + repeat count, plus the actual constraint ("do not re-propose; if you revisit one, state inPROCESS.mdwhat is materially different and which lesson it counters").Wired into
_augment_instructions— the one function whose output reaches the prompt (#114) and which all three algorithms route through, so hill-climb / GEPA / SkillOpt get it with zero per-algorithm plumbing. Every rejection site now recordsapproach:run_step, GEPA's local minibatch gate, GEPA's val gate, and GEPA's two merge gates.Per the task brief I used
RunDir.iteration_events()semantics throughout and hand-filtered no event kinds —dead_end_constraintsreadsrejected.jsonl, which is kind-agnostic by construction, so #216's class of bug cannot recur here.How constraints are bounded
Three independent budgets, zero LLM calls (pure Python, per PR #205 — no
aux_modelneeded; a verbatim diff signature is more actionable than a paraphrase anyway):_MAX_DEAD_ENDS_MAX_APPROACH_CHARS, enforced on write and on readdead_end_constraintsEviction policy is recency, not relevance: the newest rejections are the ones the current lineage is closest to re-proposing, and recency needs no scoring model. Repeats are counted, not stored twice — 50 re-proposals of one idea is one row saying "re-proposed 50x".
Also:
render_instructionscapped its own output, but_augment_instructionsappends after it — so the cross-iteration blocks were previously outside the ceiling.cap_instructionsis extracted so the final assembled prompt is held underMAX_INSTRUCTIONS_CHARS(60k).Enforcement: advisory at the prompt, hard at the gate
Stated precisely because it matters: cap-evolve cannot forbid a black-box agent CLI from re-emitting an edit. The constraint is prompt text. What is hard is the val gate — a re-proposed dead end is still rejected, and the repeat is counted and shown back ("re-proposed 4x"), which is a strictly stronger signal each time.
RUN.mdnow says exactly this rather than implying a guarantee.Drive-by root-cause fix: one scratch list, four consumers
Building the signature surfaced a real bug.
cache._IGNORE_NAMES,gepa._NON_COMPONENT,harness._CAP_DIFF_SKIPanddashboard._DIFF_SKIPeach kept their own copy of "framework scratch, not capability" — and they had drifted. Only GEPA's knew aboutFOCUS.md/REFLECTION.md, so a GEPA candidate's "capability diff" reported its reflective scratch as a real edit everywhere that diff is shown (dashboard, RUNMAP,prior_iterations/). First GEPA e2e run made this visible: the signature was 300 chars ofFOCUS.md/REFLECTION.mdboilerplate with the real edit truncated off. Unified asoptimizer_context.SCRATCH_NAMES. Fixed once, where all four consumers route through.Scope
Rejected-approach constraints only. Not #128 (synthesized insight/priors) and not #130 (plateau detection).
Dashboard
Insights "What not to try" now shows the rejected edit signature under each reason — same data, one new optional field, degrades cleanly on pre-#129 runs.
Expected merge order
origin/main→ #199 (issue #109) → #212 (issue #114) → this. One trivial conflict resolving #199+#212 ingepa.py(keep #199'srender_instructions, #212's 3-arg_augment_instructions); already resolved in the merge commit on this branch.Verification
Merged-tree baseline (main + #199 + #212): 200 passed. This branch: 213 passed, 0 failed (+13 new tests in
core/tests/test_failure_memory.py).compileall core skillsclean.Fail-before, proven by stashing the implementation and keeping only the test file:
Real end-to-end, zero API cost — all three algorithms
examples/toy_calcviacap-evolve run,mockoptimizer, 5 iterations,stall: 99so rejections accumulate. Mock script proposes a deliberately harmful edit, so every candidate is genuinely rejected and the mock re-proposes the same dead end.hill-climb, iteration 5 prompt:
GEPA — specifically proven, being the algorithm whose cross-iteration channel was silently empty before #199:
SkillOpt:
Prompt stays capped; no sealed-test/val leak
test_constraints_bounded_on_a_long_runadditionally proves 50 rejections with 5 KB signatures and 5 KB reasons produce a block < 8 KB, not 50 verbatim constraints.Full commands + output in the
## 🔬 Evidencecomment below.