Skill lifecycle phase 3: reflection rides the persistence branch - #623
Merged
Merged
Conversation
Phase 3 of docs/design-docs/skill-lifecycle.md — the outcome-to-skill pump. One deliberate divergence from the doc: no separate reflection branch. The persistence branch is already the silent replay-fork this needs, so reflection rides it as a single unified pass (the shape the Hermes reference implementation runs in production). - Channel accumulates a reflection signal: a turn crossing min_tool_iterations tool calls, or a worker completing successfully. Cron conversations never reflect. - When the signal is set and the per-conversation cooldown allows, the next persistence branch spawns with skill_reflection: its prompt gains the reflection section and its toolset gains read_skill/skill_manage/ skills_list under WriteOrigin::Agent, so every phase-2 rail applies. The signal alone can spawn the pass; passes without it are unchanged. - Reflection prompt section: decide-first (no writes is the common case), the patch-over-create preference ladder, and the negative- capture bans (no environment flukes, no negative tool claims, no transient errors, no one-off narratives). - [skills.reflection] config: enabled / min_tool_iterations (10) / cooldown_secs (3600), defaults + per-agent override, hot-reloadable.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 3 of
docs/design-docs/skill-lifecycle.md: the pump that turns outcomes into skills. Stacked on #622.One deliberate divergence from the design doc, per discussion: no separate reflection branch. The memory persistence branch already is the replay-fork this phase needs — silent, history-cloning, output-suppressed — so reflection rides it as one unified pass instead of spawning a second silent branch after heavy turns. This is also the shape Hermes actually runs in production (one post-task review fork handles memory and skills together).
How it works
The channel accumulates a reflection signal from two sources:
min_tool_iterationstool calls (counted from the appended history — one tool-call assistant message per iteration), orWhen the signal is set and the cooldown allows, the next persistence branch spawns with
skill_reflectionon: its prompt gains the reflection section and its toolset gainsread_skill/skill_manage/skills_listunderWriteOrigin::Agent— so every Phase 2 rail applies mechanically (workspace-only, no installed or pinned targets, read-before-write, delete archives). The reflection signal can spawn the pass on its own even when no memory trigger fired, and cron conversations never reflect — scheduled runs repeat the same procedure on a timer and would grind out noise skills.Persistence passes without the signal are byte-for-byte unchanged, and ingestion's persistence branch never reflects.
The prompt is the policy
The reflection section carries the rules that matter:
references/file → only then create, named for the task class, never the incidentConfig
Follows the defaults + per-agent override pattern, hot-reloadable through
RuntimeConfiglike everything else.Testing
Tool-call counting over history slices, the reflection section gating in the rendered prompt, and the TOML merge for the new
[skills.reflection]block.Note
This PR implements skill reflection on the memory persistence branch, triggered when turns exceed
min_tool_iterationstool calls or workers complete successfully. The reflection signal is consumed by the next persistence pass, which now renders an additional reflection prompt section and gains skill management tools under agent-origin rails. Key changes: reflection signal tracking inChannel, tool-call counting inchannel_history, prompt rendering for conditional skill reflection inmemory_persistence.md.j2, and new reflection config block intoml_schema.rs. Cron conversations never reflect to avoid noise. Written by Tembo for commit 44edee99. This will update automatically on new commits.