Skip to content

fix: keep Hermes session importer tests isolated - #179

Open
ideas24h wants to merge 5 commits into
NousResearch:mainfrom
ideas24h:fix/pr178-hermes-session-importer-green
Open

ideas24h wants to merge 5 commits into
NousResearch:mainfrom
ideas24h:fix/pr178-hermes-session-importer-green

Conversation

@ideas24h

Copy link
Copy Markdown

Summary

Validation

Context: #178 failed locally because SQLite-focused tests fell back into ~/.hermes/sessions and hit a corrupt legacy JSON file.

joshpineda17 and others added 5 commits August 24, 2026 09:54
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.
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