feat(sleep): consolidate skill groups independently with failure isolation - #186
feat(sleep): consolidate skill groups independently with failure isolation#186bogdanbaciu21 wants to merge 1 commit into
Conversation
…ation Add consolidate_groups: each skill group runs through the existing consolidate() with its own tasks, document, and gate decision. Task-less, name-less, repeated, and raising groups are reported instead of aborting the night, shared memory is read-only, and adoption stays explicit. Refs microsoft#120
There was a problem hiding this comment.
Pull request overview
This PR introduces a new per-skill-group consolidation driver for SkillOpt-Sleep so multiple skill documents can be consolidated independently in one “night,” with failures isolated per group (supporting the broader multi-skill routing work in #120).
Changes:
- Add
skillopt_sleep/multi_skill.pywithSkillGroup,GroupConsolidation,consolidate_groups(), andaccepted_group_skills()to run consolidations per group and report per-group outcomes. - Add
tests/test_sleep_multi_skill.pyto validate skip/duplicate/failure isolation behavior and accepted-only extraction.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| skillopt_sleep/multi_skill.py | Implements group-wise consolidation orchestration and per-group outcome reporting. |
| tests/test_sleep_multi_skill.py | Adds unit tests covering multi-group behavior, skips, duplicates, isolation, and accepted filtering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| result = consolidate_fn( | ||
| backend, list(group.tasks), group.skill, memory, | ||
| evolve_memory=False, **consolidate_kwargs, | ||
| ) |
| Returns one entry per input group, keyed by skill name and ordered | ||
| first-seen. A group is ``skipped`` when it is unusable (blank name, no | ||
| tasks, repeated name) and ``failed`` when its own consolidation raised; both | ||
| leave every other group's decision untouched. |
| with tempfile.TemporaryDirectory() as tmp: | ||
| before = sorted(os.listdir(tmp)) | ||
| consolidate_groups( | ||
| MockBackend(), | ||
| [SkillGroup("research-skill", set_learned("", []), _tasks(researcher_persona))], | ||
| edit_budget=4, night=1, | ||
| ) | ||
| self.assertEqual(sorted(os.listdir(tmp)), before) |
|
Thanks for the welcome on #120. To keep review load bounded and follow the incremental shape Yif-Yang suggested, I'm closing this PR for now and will reopen it once #182 (the harvesting slice) lands or the maintainer asks for the next slice. The branch stays on my fork so this can be re-opened as-is. Happy to restructure if a different cadence is preferred. |
Summary
Fifth review-sized slice of #120: consolidate several skill groups in one night
without letting one bad group take the others down.
Adds
skillopt_sleep/multi_skill.py:SkillGroup(skill_name, skill, tasks)— one skill's slice of the night;consolidate_groups(backend, groups, memory="", *, consolidate_fn=consolidate, **kwargs)runs each group through the existing
consolidate()independently and returnsa first-seen-ordered
{skill_name: GroupConsolidation};GroupConsolidationcarriesstatus(consolidated/skipped/failed),the
ConsolidationResultwhen the group ran, and areasonwhen it did not;accepted_group_skills(outcomes)→{skill_name: new document}for groupswhose gate accepted an update.
Isolation rules: a group with no tasks or no name is
skippedwith a reason, arepeated name runs once (first wins), and a group whose consolidation raises is
failedwith the exception type/message — every other group still gets its owngate decision and document. Shared memory is passed through read-only
(
evolve_memory=False), so no group can rewrite another group's memory.Nothing existing changes:
cycle.pyandconsolidate()are untouched, so thecurrent single-managed-skill night behaves exactly as before, and adoption stays
explicit — this module writes no files.
Tests
python -m pytest -q tests/test_sleep_multi_skill.py→ 10 passedpython -m pytest -q→ 566 passed, 7 skipped (basemainat8304e6c:556 passed, 7 skipped)
python -m ruff check skillopt_sleep/multi_skill.py tests/test_sleep_multi_skill.py→ All checks passed
Covered: empty input; a single group producing byte-identical results to calling
consolidate()directly; two groups keeping their own documents; task-less,name-less, and repeated groups; a raising group isolated while its neighbour
still accepts; shared memory not evolved; accepted-only filtering; and no files
written.
Refs #120