Skip to content

feat(principle-distiller): config-gated Phase 3 conversation_loop integration - #78003

Open
12312ewqdq wants to merge 4 commits into
NousResearch:mainfrom
12312ewqdq:feat/principle-distiller-phase3
Open

feat(principle-distiller): config-gated Phase 3 conversation_loop integration#78003
12312ewqdq wants to merge 4 commits into
NousResearch:mainfrom
12312ewqdq:feat/principle-distiller-phase3

Conversation

@12312ewqdq

Copy link
Copy Markdown

Phase 3 principle distiller — config-gated conversation_loop hook

Recovered from hermes-update-autostash-20260803-104527 (worker crash during autostash) and independently re-validated. Four commits on feat/principle-distiller-phase3 (base: current origin/main):

  • feat(principle-distiller): restore Phase 3 integration — lazy defensive import of auto/principle_distiller; config gate principle_distiller.enabled (default false) + HERMES_PRINCIPLE_DISTILLER env override, read once at loop startup; turn-start principle retrieval/stash for the reward hook (W1); turn-end distill with validated dict/text append to final response (D1); tool-error marker scan; exception containment on every path.
  • test(principle-distiller): add enabled/disabled smoke tests — full-loop smoke coverage.
  • test(principle-distiller): config-key e2e switch coverage + off-spelling coercion.
  • fix(principle-distiller): scope-block marker gap + isinstance guard (sibling task t_d1048be1 advisory Terminal tool #1/Support passing morph snapshot id #2) — substring marker for the tool_search scope-block error plus isinstance(original_user_message, str) non-empty guard; 6 pure-function unit tests for _turn_slice_has_tool_error.

Validation

New tests (59 total, all passing)

File Count Covers
tests/hermes_cli/test_principle_distiller_config.py 29 default-disabled, env-override truthy/falsy matrix (1/true/TRUE/True/yes/on/" ON "/empty), config-key precedence, malformed-section degradation, hermes config set round-trip, off-spelling coercion
tests/integration/test_conversation_loop_distiller_enabled.py 4 clean turn distills+appends+persists, exact turn-state passed to distiller, tool-error flag, two-turn reward-hook strong-correction delta
tests/integration/test_conversation_loop_distiller_robustness.py 17 disabled path never executes distiller, distiller exception contained, malformed outputs (str/list/{}/missing/non-str text) ignored, reward-hook exception contained, injection failure contained, module-unavailable skip, _turn_slice_has_tool_error pure-function cases (startswith markers, mid-content substring, normal content not flagged)
tests/integration/test_principle_distiller_smoke.py 2 enabled/disabled smoke over the full loop
tests/integration/test_conversation_loop_distiller_config_switch.py 7 config-unset preserves prior behavior, false/off keep disabled, true reaches integration point, invalid values degrade per config validation

Edge cases covered: env-override precedence (incl. whitespace " ON " and empty string), malformed config section degrades to false without raising, disabled path byte-identical to model output, distiller exceptions logged at ERROR and loop continues, every malformed return shape is ignored and never leaks into the response, reward-hook failures contained, missing auto/ module degrades to skip even when enabled, non-str/empty user_message falls back to the raw turn message.

Exact test command

python -m pytest -o addopts="" \
  tests/hermes_cli/test_principle_distiller_config.py \
  tests/integration/test_conversation_loop_distiller_enabled.py \
  tests/integration/test_conversation_loop_distiller_robustness.py \
  tests/integration/test_principle_distiller_smoke.py \
  tests/integration/test_conversation_loop_distiller_config_switch.py

Result: 59 passed in 91.84s.

The new tests are hermetic (in-process mock HTTP provider, zero network/LLM calls) and carry no integration marker, so they also collect under the repo's default -m 'not integration' deselect (verified: 29 collected with default addopts; 9 collected from smoke+config-switch with default addopts). -o addopts="" is used only for an explicit, reproducible command.

Runtime regression

Scoped to the code paths the feature touches (tests/agent turn-context/loop/finalizer set — 8 files):

python -m pytest tests/agent/test_turn_context.py \
  tests/agent/test_empty_tool_name_loop_dampening.py \
  tests/agent/test_crossloop_client_cache.py \
  tests/agent/test_turn_retry_state.py tests/agent/test_turn_summary.py \
  tests/agent/test_turn_finalizer_final_response_persistence.py \
  tests/agent/test_turn_finalizer_iteration_limit_exit.py \
  tests/agent/test_turn_finalizer_cleanup_guard.py -q

Result: 45 passed in 29.49s.

The full tests/agent suite exceeds this Windows environment's time budget (~10 min and still running), so the run was scoped. Full-suite collection is at parity with the pre-feature baseline (aad8f7412): the same 3 pre-existing gateway/relay collection errors (tests/gateway/test_teams.py, relay/test_relay_going_idle.py, relay/test_ws_transport.py) on both trees — no new collection failures introduced.

Hermes added 4 commits August 4, 2026 04:03
… 59142c9b2

- Lazy import principle_distiller with sys.path setup + exception degradation
- Config gate: principle_distiller.enabled (default false) + HERMES_PRINCIPLE_DISTILLER env override, read once at loop startup
- Turn start: retrieve principles, stash hits for reward hook (W1)
- Turn end: distill + append to response, validated dict/text (D1)
- Tool-error markers scan + _turn_had_tool_error flags
- 41 tests passing (config, enabled-path, robustness)
- Restored from hermes-update-autostash-20260803-104527 (stash@{0}), worker crash recovery
…integration

Two one-turn end-to-end checks through AIAgent.run_conversation against the
in-process mock provider, reusing the harness from the sibling distiller
suites:
- enabled: seeded store + HERMES_PRINCIPLE_DISTILLER=1 -> distilled principle
  appended to final_response and persisted (source self-distilled), gate flag
  stashed, W1 injection on the wire;
- disabled: byte-identical response, no stashes, store untouched, no wire
  injection.
…t_d1048be1 NousResearch#1/NousResearch#2)

- _TOOL_ERROR_CONTENT_CONTAINS: substring scan catches tool_search
  scope-block error ("'<name>' is not available in this session. Use
  tool_search ...") which starts with a quote, evading all startswith
  markers; keeps prefix semantics for the original 6 markers (no
  false-positive on 'Tool ' inside normal content)
- isinstance(original_user_message, str) and non-empty guard on the
  distiller user_message input (empty/non-str fallback to user_message)
- 6 pure-function unit tests for _turn_slice_has_tool_error (previously
  zero coverage); all 59 distiller tests pass
@12312ewqdq

Copy link
Copy Markdown
Author

Local validation scope (Windows host)

All verification was done on this Windows host before opening the PR:

  • New tests: 59/59 passed across the 5 distiller test files (91.84s), run with python -m pytest -o addopts="" <files>. The -o addopts="" override is only because the repo default -m 'not integration' deselects integration-marked tests; these tests are hermetic (in-process mock HTTP provider, zero network/LLM) and also collect under the default config (verified).
  • Runtime regression: scoped to the code paths the feature touches (8 turn/loop/finalizer files in tests/agent) — 45/45 passed (29.49s). The full 25k-test suite exceeds this environment's time budget; full-suite collection is at parity with the pre-feature baseline (aad8f74): the same 3 pre-existing gateway/relay collection errors on both trees, no new collection failures introduced.
  • CI on this PR is the definitive remaining gate for the full suite.

@alt-glitch alt-glitch added type/feature New feature or request comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 3, 2026
@12312ewqdq

Copy link
Copy Markdown
Author

Rebase & post-rebase validation complete — awaiting orchestrator decision (no push performed)

Post-rebase HEAD

  • Local branch tip (feat/principle-distiller-phase3): a9ee317f4f96411f0ffb0f2ecbbd3431cdd0bc4a (5 commits: 4 replayed distiller commits + 1 importorskip hardening commit a9ee317f4)
  • Remote PR head is unchanged (abd95793cb28a409adb4fefa3ee1d4251e715073) — no force-push was performed, by design.

Conflict-resolution summary (rebase onto 42708f8bb = origin/main at rebase time)

  • Replayed the 4 distiller commits (23aaff214, 58141d82a, e71cab5f9, 1b310b932); dropped the redundant docs commit a991dfc25.
  • Resolved conflicts manually in agent/conversation_loop.py, agent/turn_context.py, hermes_cli/config*.py, website/docs/user-guide/configuration.md.
  • For the distiller-unrelated qwen3.8-max files (agent/model_metadata.py, hermes_cli/models.py, website/static/api/model-catalog.json) resolved in favor of main — zero drift vs base.
  • Two further origin/main advances during the run were re-anchored with zero conflicts; one self-inflicted fake conflict was aborted cleanly with no impact on the final tree.
  • Added a9ee317f4: 9x pytest.importorskip("auto.principle_distiller") guards in the 3 integration test files so CI environments without the optional module skip cleanly instead of erroring.
  • Acceptance gate against rebase base 42708f8bb: exactly 11 changed paths (6 M + 5 A), 0 merges, git diff --check clean.

Rerun test results (isolated worktree, live venv python 3.13.14)

Suite Result
59 new distiller tests (5 PR files) 59 passed in 87.04s (0 skip / 0 fail)
45 regression tests (8 tests/agent files) 45 passed in 31.87s
tests/hermes_cli config tests (7 files) 96 passed, 1 failed in 15.06s

The single failure — tests/hermes_cli/test_config.py::TestGetHermesHome::test_default_path — is pre-existing and unrelated to this PR: it asserts the POSIX default ~/.hermes on all platforms, while hermes_constants.get_hermes_home() returns the documented Windows-native default %LOCALAPPDATA%\hermes. Reproduces identically on a clean checkout of the rebase base 42708f8bb; hermes_constants.py / test_config.py are byte-identical base..HEAD.

Merge state

  • gh pr view 78003 --json mergeStateStatusUNKNOWN (remote head not updated — will recompute after push)
  • REST API: mergeable: true, mergeable_state: **blocked** — expected BLOCKED until the orchestrator approves the force-push/update, per plan.

⚠️ Caveat for the orchestrator

origin/main has since advanced to 3fa318a50 (merge of #68881, relay shared-metrics). That commit does not touch any of the PR's 11 files, but the branch is now 1 commit behind current main — force-pushing a9ee317f4 as-is would present a 27-path diff vs current origin/main (16 of those belong to main). A final git rebase --onto origin/main 42708f8bb feat/principle-distiller-phase3 (zero expected conflicts) + quick test re-run is recommended before force-push. Decision left to the orchestrator; no push, no merge performed here.

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

Labels

area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants