Skip to content

fix: Phase 1 skill evolution is non-functional (GEPA output discarded, sessiondb mines 0 Hermes messages) - #178

Open
josuedpinedad wants to merge 4 commits into
NousResearch:mainfrom
josuedpinedad:fix/phase1-skill-evolution-nonfunctional
Open

josuedpinedad wants to merge 4 commits into
NousResearch:mainfrom
josuedpinedad:fix/phase1-skill-evolution-nonfunctional

Conversation

@josuedpinedad

Copy link
Copy Markdown

Summary

Phase 1 (skill evolution) cannot currently produce an evolved skill. I hit four
independent blocking defects trying to evolve a single skill end to end, plus a
Windows-only test failure. Each fix is a separate commit; the suite goes from
144 passed / 1 failed → 156 passed / 0 failed.

The headline bug is #1: GEPA works, and its output is discarded.

1. The skill text is not an optimizable parameter (skill_module.py)

SkillModule passed the skill body as a dspy InputField (skill_instructions).
DSPy optimizers only rewrite a predictor's signature instructions — they
never touch input values. So the skill text was never mutated.

On a real 6-iteration run, GEPA logged 7 accepted proposals for
predictor.predict:

INFO dspy.teleprompt.gepa.gepa: Iteration 1: Proposed new text for predictor.predict: ...
INFO dspy.teleprompt.gepa.gepa: Iteration 2: Proposed new text for predictor.predict: ...

…and the result was:

{ "baseline_score": 0.4302521929824561,
  "evolved_score":  0.4302521929824561,
  "improvement": 0.0,
  "baseline_size": 5433,
  "evolved_size":  5433 }

diff baseline_skill.md evolved_skill.mdbyte-identical. Every proposed
improvement was thrown away.

Fix: seed the signature via TaskWithSkill.with_instructions(skill_text) so the
skill body is the optimizable parameter, and expose skill_text as a property
reading it back. Callers are unchanged.

SkillModule had no tests, which is why this went unnoticed. Added
TestSkillModuleIsOptimizable, which fails on the old wiring.

2. --eval-source sessiondb mines 0 messages from Hermes (external_importers.py)

HermesSessionImporter only reads ~/.hermes/sessions/*.json. Modern Hermes
stores transcripts in a single SQLite state.db (sessions + messages
tables), so the richest of the three importers silently contributed nothing:

Importing from Claude Code...
  Found 857 messages
Importing from Copilot...
  Found 0 messages
Importing from Hermes Agent...
  Found 0 messages      # <-- 8,240 messages / 52 sessions sat in state.db

After the fix: 424 user/assistant pairs from the same machine.

  • _candidate_db_paths() honors HERMES_HOME, then per-platform defaults
    (Windows %LOCALAPPDATA%\hermes, XDG, macOS Application Support, ~/.hermes).
  • _extract_from_sqlite() opens the DB read-only (file:...?mode=ro) so a
    live Hermes process is never disturbed; pairs each user turn with the next
    assistant turn within the same session; skips compacted rows so
    compression artifacts don't become training data.
  • Cron-injected preambles are skipped — agent scaffolding, not user tasks.
  • Legacy JSON reader retained as a fallback.
  • STATE_DB added for explicit override (also lets tests pin the DB rather than
    picking up whatever install exists on the host).

3. dspy.GEPA(max_steps=...) doesn't exist in dspy 3.x (evolve_skill.py)

Every run raised TypeError: GEPA.__init__() got an unexpected keyword argument
and fell through to the MIPROv2 fallback — GEPA never ran. The fallback then
died on ImportError: MIPROv2 requires optional dependency 'optuna', taking the
whole run with it.

Now uses max_full_evals=iterations plus the reflection_lm GEPA requires, and
declares optuna under a mipro extra so the documented fallback works.

Also added _gepa_metric, an arity-tolerant wrapper: GEPA calls metrics as
(gold, pred, trace, pred_name, pred_trace) while Evaluate uses
(gold, pred), and skill_fitness_metric accepts only three positional args.

4. The structural constraint can never pass, blocking deploys (evolve_skill.py)

_check_skill_structure asserts YAML frontmatter exists, but was handed
skill["body"] — which load_skill() has already stripped of frontmatter:

✗ skill_structure: Skill missing: YAML frontmatter (---), name field, description field

On the baseline that's a spurious warning; on the evolved artifact it triggers
✗ Evolved skill FAILED constraints — not deploying, discarding good variants.
Now validates the full artifact (skill["raw"] / evolved_full).

5. Windows-only test failure (test_config_repo_path.py)

test_resolve_expands_user_home monkeypatched only HOME, but
Path.expanduser() reads USERPROFILE on Windows. Fails on main on any
Windows machine, independent of this PR.

Test Plan

  • pytest tests/ -q156 passed (from 144 passed / 1 failed on main)
  • 11 new tests: SQLite importer (pairing, session isolation, secret
    filtering, limit, cron skip, legacy fallback, unrelated schema) +
    SkillModule optimizability
  • TestSkillModuleIsOptimizable verified to fail against the old
    InputField wiring — it is a real regression guard, not a tautology
  • Existing legacy-JSON importer tests isolated from the host's real
    state.db and still passing
  • Full end-to-end run on a real skill after the fixes: 6 iterations, 441s,
    completes and produces a genuinely different evolved skill, correctly
    gated by growth_limit (+22.9% vs the +20% cap) instead of silently
    returning the baseline

Notes

Verified against dspy 3.3.1 / gepa 0.1.4 on Windows with Python 3.14. Fixes
1 and 2 are the load-bearing ones — without them Phase 1 either evolves nothing
or has nothing to evolve against.

One observation worth flagging for anyone using this: with a small mined dataset
(11 examples, mostly one input shape) GEPA overfits hard — in my run it
replaced a general ## Diagnosis Workflow section with a narrow variant
specific to the dominant input type. The growth_limit guardrail happened to
catch it. Reviewing the diff before adopting evolved_skill.md is essential;
that might be worth a line in the README.

SkillModule passed the skill body as a dspy InputField
(`skill_instructions`) while DSPy optimizers only rewrite a predictor's
*signature instructions* — they never touch input values. The skill text
was therefore never mutated: GEPA proposed candidates every iteration,
`optimized_module.skill_text` returned the untouched original, and the
"evolved" skill was written to disk byte-identical to the baseline.

Observed on a real run: GEPA logged 7 accepted proposals for
`predictor.predict`, and the result was `improvement: 0.0` with
`evolved_size == baseline_size`. Phase 1 could not produce an evolved
skill at all.

Seed the signature with `TaskWithSkill.with_instructions(skill_text)` so
the skill body *is* the optimizable parameter, and expose `skill_text` as
a property reading it back from the predictor. Callers are unchanged.

Add TestSkillModuleIsOptimizable, which fails on the old wiring: it
mutates the predictor's instructions the way an optimizer does and
asserts `skill_text` follows. SkillModule previously had no tests.
HermesSessionImporter only understood `~/.hermes/sessions/*.json`, a
layout modern Hermes no longer writes. Transcripts now live in a single
SQLite database (`state.db`) with `sessions` and `messages` tables, so
`--eval-source sessiondb` silently mined **0 messages** from Hermes on
any current install — the one source that has richer signal than Claude
Code (user-only) or Copilot.

On the reporting machine: `~/.hermes/sessions/` did not exist, while
`state.db` held 8,240 messages across 52 sessions. After this change the
Hermes importer yields 424 user/assistant pairs where it previously
yielded none.

- Add `_candidate_db_paths()`: honors `HERMES_HOME`, then per-platform
  defaults (Windows `%LOCALAPPDATA%\hermes`, XDG, macOS Application
  Support, `~/.hermes`).
- Add `_extract_from_sqlite()`: opens the DB read-only (`mode=ro`) so a
  live Hermes process is never disturbed, pairs each user turn with the
  next assistant turn *within the same session*, and skips rows flagged
  `compacted` so compression artifacts don't become training data.
- Skip cron-injected preambles — agent scaffolding, not user tasks.
- Keep the legacy JSON reader as a fallback so older installs still work.
- Add `STATE_DB` for an explicit override, which also lets tests pin the
  database instead of picking up whatever install exists on the host.

Tests: add TestHermesSessionImporterSQLite (pairing, session isolation,
secret filtering, limit, cron skip, legacy fallback, unrelated schema),
and isolate the existing legacy-JSON tests from the host's real state.db.
…check

Three defects in the Phase 1 driver, each of which alone prevents a run
from producing a deployable skill:

1. `dspy.GEPA(max_steps=...)` — no such argument in dspy 3.x. Every run
   raised TypeError and fell through to the MIPROv2 fallback, so GEPA
   never actually ran. Use `max_full_evals=iterations` (the closest
   analogue to "iterations") and pass the `reflection_lm` GEPA needs to
   propose mutations.

2. The MIPROv2 fallback then died on `ImportError: MIPROv2 requires
   optional dependency 'optuna'`, taking the whole run down. With GEPA
   fixed the fallback is rare, but `optuna` is now declared so the
   documented fallback path works instead of crashing.

3. `_check_skill_structure` asserts YAML frontmatter exists, but was
   handed `skill["body"]` — which `load_skill()` has already stripped of
   frontmatter. The check could therefore never pass. On the baseline it
   printed a spurious violation; on the evolved artifact it tripped
   "Evolved skill FAILED constraints — not deploying", discarding good
   variants. Validate the full artifact (`skill["raw"]` / `evolved_full`)
   so the structural check sees the frontmatter it is asserting on.

Also add `_gepa_metric`, an arity-tolerant wrapper: GEPA invokes metrics
as `(gold, pred, trace, pred_name, pred_trace)` while Evaluate uses
`(gold, pred)`, and `skill_fitness_metric` accepts only three positional
args. Normalizing at the call site keeps the metric's public signature
intact.

After these fixes a 6-iteration run completes end to end and produces a
genuinely different evolved skill, correctly gated by the growth limit.
`test_resolve_expands_user_home` monkeypatched only HOME, but
`Path.expanduser()` reads USERPROFILE (and HOMEDRIVE/HOMEPATH) on
Windows. The test asserted against `/home/example/...` while expansion
resolved to the real user profile, so it failed on every Windows run
independently of any of the fixes in this PR.

Set USERPROFILE alongside HOME and clear HOMEDRIVE/HOMEPATH so the test
exercises tilde expansion on both platforms.
@josuedpinedad
josuedpinedad force-pushed the fix/phase1-skill-evolution-nonfunctional branch from 79b4a1e to 3247dad Compare August 24, 2026 16:03
numandev1 added a commit to numandev1/hermes-agent-self-evolution that referenced this pull request Aug 27, 2026
Adopted from upstream review of NousResearch#178
and NousResearch#179, which independently found the same core defects this fork already
fixed but also caught data-quality problems it had not.

Hermes writes machine text into the `user` role, and mining it verbatim turns
agent plumbing into evaluation tasks. Four kinds found on the live install:

  - [CONTEXT COMPACTION - REFERENCE ONLY] summary blocks
  - [IMPORTANT: ...scheduled cron job...] delivery preambles wrapping the
    real instruction
  - [IMPORTANT: ...invoked the "x" skill] preambles followed by the entire
    SKILL.md - the worst case, since the optimizer would be trained on the
    artifact it is meant to improve, with the skill file as the request
  - [ASYNC DELEGATION COMPLETE] subagent notices

Peeling is iterative, because these layer: a preamble wraps a skill file, so
the checks have to re-run on whatever a strip exposes. Bracket matching is
nesting-aware, because the cron preamble quotes "[SILENT]" inside itself and
a scan for the first ] closed on that, leaving boilerplate behind as the task.

Deliberately NOT adopting NousResearch#178's filter on the `compacted` flag. That flag
means "rolled out of the active context window", not "machine-written": 900 of
1,290 user rows in one profile are compacted and nearly all are genuine asks
("audit this website"), so filtering on it discards most of the corpus while
still admitting the injected blocks. Content-based detection does the job the
flag was being used to approximate.

Also adopted:
  - UnicodeDecodeError handling in the legacy JSON importer (NousResearch#179) - it is
    neither JSONDecodeError nor OSError, so one non-UTF8 file aborted the
    whole import
  - platform-aware install discovery: %LOCALAPPDATA%, XDG_DATA_HOME, macOS
    Application Support (NousResearch#178)
  - USERPROFILE alongside HOME in path tests; Path.expanduser() reads the
    former on Windows, so these failed there regardless of the code (NousResearch#178)

Live result: 2,127 -> 1,989 mined pairs, with zero scaffolding remaining.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants