feat(sleep): propagate harvested skill hints into mined tasks

[bogdanbaciu21]

Summary

Second review-sized slice of #120. Mined tasks currently carry no notion of
which skill they exercised, so per-skill work later in the issue has nothing to
group on.

• add an optional, default-empty TaskRecord.skill_hint with backward-compatible
  serialization (to_dict() / from_dict() still accept legacy payloads);
• add mine.session_skill_hint(digest): a session that invoked exactly one skill
  names it, while zero skills (unknown) and several skills (ambiguous) both
  return "";
• propagate the hint in the deterministic miner and in the LLM miner's rule and
  rubric tasks;
• in dedup_tasks, fill a missing hint from a merged duplicate but clear the
  hint when merged sessions disagree.

Nothing consumes skill_hint yet: grouping, staging, and adoption behavior are
unchanged, so absent or ambiguous hints stay in the existing catch-all path.

Stacked on #182 (skoc-001-harvest-claude-skill-tool-use), which adds
SessionDigest.skills_used. This branch contains that commit as its parent;
please review/merge #182 first and this diff reduces to the skill_hint commit
alone. Happy to rebase or squash into whatever shape you prefer.

Tests

• python -m pytest -q tests/test_sleep_skill_hint.py → 13 passed
• python -m pytest -q → 580 passed, 7 skipped (base main at 8304e6c:
  556 passed, 7 skipped)
• python -m ruff check skillopt_sleep/types.py skillopt_sleep/mine.py skillopt_sleep/llm_miner.py tests/test_sleep_skill_hint.py
  reports only the three pre-existing F401 findings already present on main.

New coverage: field default, serialization round-trip, legacy payload without
skill_hint, single/absent/ambiguous/repeated skill sessions, heuristic-mine
propagation, dedup agreement/fill/conflict, and LLM-miner rule and rubric tasks.

Refs #120

[Copilot]

Pull request overview

Adds end-to-end propagation of harvested Claude Skill tool invocations into mined TaskRecords as a skill_hint, enabling later per-skill grouping without changing existing behavior for unknown/ambiguous sessions.

Changes:

• Introduces SessionDigest.skills_used harvesting from Claude Skill tool-use blocks and persists it via existing to_dict() serialization.
• Adds TaskRecord.skill_hint plus mine.session_skill_hint() and propagates the hint through both heuristic and LLM-backed miners.
• Extends dedup_tasks() to merge/resolve skill_hint across duplicate tasks, and adds unit tests covering defaults, legacy payloads, propagation, and dedup behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
skillopt_sleep/types.py	Adds skills_used to SessionDigest and skill_hint to TaskRecord with backward-compatible loading.
skillopt_sleep/harvest.py	Extracts Skill tool targets from assistant tool-use blocks into SessionDigest.skills_used.
skillopt_sleep/mine.py	Adds session_skill_hint() and propagates skill_hint into heuristic mining and dedup logic.
skillopt_sleep/llm_miner.py	Propagates skill_hint into LLM-mined rule/rubric tasks via session_skill_hint().
tests/test_sleep_skill_hint.py	Adds unit tests for skill_hint defaults/serialization, propagation, and dedup agreement/fill/conflict.
tests/test_sleep_skill_harvest.py	Adds unit tests for harvesting skills_used from Claude transcripts, including malformed cases.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Yif-Yang]

Thank you for building this as a follow-up to #182, which is now merged. Propagating a skill hint into mined tasks is useful, but the current head has one deterministic correctness bug and one propagation gap that should be fixed before merge.

1. dedup_tasks() uses an empty string for both “no hint seen yet” and “a conflict was already observed.” This makes the result order-dependent: A, B, A ends with A, while A, A, B ends empty. Please aggregate the non-empty hints for each task ID and retain a hint only when the final set has exactly one value.
2. dream_augment() and recall_similar() reconstruct TaskRecord objects without carrying skill_hint, so the metadata is lost in those paths.

Please add permutation tests such as A/B/A, B/A/B, and unknown/empty interleavings, plus focused dream/recall propagation tests. It would also be safer to place the new defaulted TaskRecord.skill_hint field at the end of the dataclass to avoid changing positional construction for external callers.

Once these focused points are addressed, this should be straightforward to re-review. Thanks again for extending the original contribution cleanly.

[bogdanbaciu21]

Will do. Super helpful feedback. Will jump on this ASAP but need some time, want to get this right and deliver good quality implementation. Will follow up when the required changes are complete. Thanks so much.

[bogdanbaciu21]

Hello. I have a proposed solution. Your insight was very helpful and informative. At this point focused tests pass, but it is late and a proper end to end full re-run would make me feel better we got this right, so will do tomorrow and hope to address and resolve this. Thx so much.

Dan

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[bogdanbaciu21]

Hi Yif,

Thank you again for the precise and very helpful review. I took some extra time with this because I wanted to address the underlying behavior properly and validate the result beyond the immediate regression cases.

Proof from the updated PR head:

116 passed, 1 skipped, and 130 subtests passed across the focused sleep path.

An exhaustive 5,460-case ordering check passed across empty, A, B, and C hints for sequence lengths one through six.

Ruff, Python compilation, and the Git diff checks all passed.

The full suite reports 585 passed and 6 skipped. The remaining two failures are macOS path-resolution assertions involving /var and /private/var; I reproduced both unchanged against the current upstream/main, so they are not regressions introduced by this update.

The core fix changes dedup_tasks() so that skill_hint is derived from the complete set of non-empty hints associated with each task ID. This removes the order dependence you identified. A/B/A and B/A/B now remain ambiguous regardless of arrival order, while empty hints remain missing information rather than becoming conflicts.

I also updated dream_augment() and recall_similar() so that they preserve the hint when constructing new TaskRecord objects. The skill_hint field is now the final field in the dataclass, preserving the positional meaning of all existing defaulted fields for external callers.

The committed regression coverage includes both alternating conflict patterns, permutations, empty-hint interleavings, an exhaustive final-set check, the dream and recall paths, and compatibility with the previous positional constructor layout.

The update is pushed in commit 74c7057 and is ready for another review whenever convenient. Thank you again for taking the time to give such specific feedback. It materially improved the implementation, and I learned quite a bit from working through the edge cases carefully.

Dan