Skip to content

Introspect should patch skills directly, not just store memories#50

Merged
MichielDean merged 10 commits into
mainfrom
feat/ll-l3g6l
May 11, 2026
Merged

Introspect should patch skills directly, not just store memories#50
MichielDean merged 10 commits into
mainfrom
feat/ll-l3g6l

Conversation

@MichielDean
Copy link
Copy Markdown
Owner

Closes droplet ll-l3g6l.

Lobsterdog Contributors added 10 commits May 11, 2026 15:19
Replaces proposed-changes.md mechanism with immediate skill file patching
when introspect produces a self_assessment with a proposed_update.

New packages and files:
- internal/skillpatch/: SkillPatcher with Patch, FindSkillFile, ValidatePatch
  methods. Patches are additive (append sections, not overwrite), idempotent,
  and gracefully degrade when SkillPatcher is nil or patching fails.

Modified packages:
- internal/paths: Added GetSkillDir() following GetDreamDiaryPath() pattern
- internal/introspect: Augmented IntrospectResult with ProposedUpdate and
  Category fields, populated from both LLM-enriched and raw paths
- internal/taxonomy: Added ParseSelfAssessmentField() utility function
- internal/dream: Added SkillPatcher to DreamerConfig, validatePatches()
  method that validates patch effectiveness (boost/flag procedure memories)
- internal/config: Added SkillPatchConfig with Dir field, NewSkillPatcher()
  helper on Config, wired through DreamerConfig
- cmd/llmem/main.go: Enhanced introspect command to call patchSkillAfter-
  Introspect() after storing self_assessment; dream command wires SkillPatcher

Test coverage:
- skillpatch_test.go: 8 test functions covering Patch, FindSkillFile,
  ValidatePatch, idempotency, frontmatter parsing, malformed files
- paths_test.go: TestGetSkillDir
- taxonomy_test.go: 3 TestParseSelfAssessmentField variants
- introspect_test.go: 4 new tests for ProposedUpdate/Category fields
- dream_test.go: 2 new tests for ValidatePatches integration
…contract, FindSkillFile docs

1. dream.go: extractBehavioralInsights now uses taxonomy.ParseSelfAssessmentField
   for exact category matching instead of strings.Contains, preventing
   double-counting when one category name appears in another memory's prose.

2. skillpatch.go: ValidatePatch converted from method to pure function.
   Was (sp *SkillPatcher) ValidatePatch(ctx, category, before, after) returning
   (PatchValidation, error) but only compared two integers — contract violation.
   Now ValidatePatch(category, before, after) returns PatchValidation, no ctx,
   no receiver, no error. Pure arithmetic clearly expressed.

3. skillpatch.go: FindSkillFile doc comment corrected to match implementation.
   Was 'searches known skill directories for a frontmatter description field'
   but implementation does categorySkillMap lookup + os.Stat. Doc now accurately
   describes the map lookup and file existence check.

Added test: TestExtractBehavioralInsights_UsesParseSelfAssessmentField
verifies no double-counting of categories mentioned in prose.
Added test: TestValidatePatch_EqualCounts_Flagged for equal counts case.
…cher, add skill patching to learnCmd

Issue 1 (tepxi): SkillPatcher.store was required by constructor but never read.
ValidatePatch was extracted to pure function; Patch/FindSkillFile do file I/O only.
Removed Store from SkillPatchConfig, SkillPatcher struct, nil check from
NewSkillPatcher, and updated config.NewSkillPatcher to not pass Store.
Removed TestNewSkillPatcher_NilStore (no longer relevant).
Removed newTestStore helper from skillpatch_test.go.

Issue 2 (mmjx3): learnCmd called IntrospectFailure but never called
patchSkillAfterIntrospect. introspectCmd patches skills, learnCmd did not.
Asymmetry fixed: added patchSkillAfterIntrospect call with same guard
(result.ProposedUpdate != "" && result.Category != "") after storing result.
…Category

IntrospectAuto now returns (IntrospectAutoResult, error) instead of (string, error),
enabling callers to patch skill files immediately after introspection when
proposed_update is procedural.

Changes:
- IntrospectAuto: return IntrospectAutoResult{MemoryID, ProposedUpdate, Category}
  instead of just string memory ID. ProposedUpdate and Category are extracted from
  LLM-enriched content via taxonomy.ParseSelfAssessmentField when available,
  empty on graceful degradation.
- main.go --auto mode: call patchSkillAfterIntrospect with result.Category and
  result.ProposedUpdate, same pattern as introspectCmd and learnCmd.
- session.go OnEndingWithIntrospect: add SkillPatcher to SessionHookConfig and
  SessionHookCoordinator, call Patch() after successful introspection when
  result has ProposedUpdate and Category.
- Updated API.md, DREAM.md, CLI.md docs to reflect new return type.
- Added TestIntrospectAuto_ResultType_ReturnsFieldsOnGracefulDegradation test.
- Updated all IntrospectAuto tests to use result.MemoryID instead of bare id.

Addresses reviewer issue: IntrospectAuto discarded ProposedUpdate and Category,
preventing skill patching after --auto introspection and session hooks.
…illpatch

Security fixes for two issues identified by security review:

1. Path traversal (blocking): sanitizeCategory validates category names
   contain only alphanumeric characters and underscores before use as
   directory names. Defense-in-depth path containment check in createSkillFile
   ensures resolved paths stay within skillDir.

2. YAML frontmatter injection (required): sanitizeYAMLValue strips newlines
   and carriage returns from category and description strings before writing
   into YAML frontmatter, preventing structure corruption.

Both issues filed as ll-l3g6l-1f1c4 and ll-l3g6l-5t8dd.
@MichielDean MichielDean merged commit ac26a18 into main May 11, 2026
2 checks passed
@MichielDean MichielDean deleted the feat/ll-l3g6l branch May 11, 2026 21:27
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.

1 participant