feat: learning system Wave 2 quality improvements

[dean0x]

Summary

Wave 2 learning system quality improvements addressing 8 issues: SessionEnd migration, procedural thresholds, transcript extraction, empty-array noise elimination, reinforcement mechanism, skill template quality, artifact path renaming, and legacy hook cleanup.

Changes

Learning Pipeline

• Move learning trigger from Stop hook → SessionEnd hook (runs at end of session, not mid-transcript)
• Implement 3-session batching for confidence aggregation
• Raise procedural thresholds to 3+ observations with 24h+ temporal spread (was 2 observations)

Transcript Extraction & Parsing

• Fix transcript string-type handling (extract content from content[0].text, not direct fields)
• Eliminate empty-array loop noise when processing short transcripts
• Improve confidence scoring for observations with varied temporal distribution

Reinforcement Mechanism

• Add local-grep-based reinforcement (no LLM call): verify artifact correctness without external API
• Build confidence for reused artifacts (skills/commands that appear in workflow multiple times)

Skill Template Quality

• Enhance skill templates with Iron Law section (immutable principle per skill)
• Add Activation section documenting when skill auto-activates
• Improve reference documentation structure

Artifact Path Migration

• Rename artifact paths: learned/ → self-learning/ (consistency with plugin structure)
• Maintain backwards-compatible detection of legacy learned/ paths during cleanup
• Clean up orphaned legacy artifacts

Legacy Cleanup

• Add backwards-compatible Stop hook cleanup
• Graceful migration for users with existing learning artifacts
• No breaking changes for new installations

Breaking Changes

None

Testing

• Transcript extraction tested with real session logs (string content type)
• Procedural threshold tested with multi-session observation sequences (24h spread)
• Reinforcement tested with grep patterns against actual learned artifacts
• SessionEnd hook tested with session completion flow (batch processing verified)
• Backwards compatibility tested with legacy learned/ path detection
• Empty-array filtering tested with short transcript edge cases

Testing gaps: End-to-end multi-session batching (requires >3 sessions in test environment). Integration testing with confidence persistence across session restarts (defer to Wave 3 if needed).

Related Issues

Closes Wave 2 quality epic (related: #160, #161)

---

Co-Authored-By: Claude noreply@anthropic.com

[dean0x]

Race Condition: Batch File Handoff Not Atomic

Lines 190-192 use cp + rm to move the session count file to the batch file. This is not atomic, creating a race window if two concurrent sessions trigger the hook simultaneously. Between the cp and rm, the second invocation could read a stale or partially-written batch file, resulting in duplicate LLM invocations or lost session IDs.

Flagged by: Security (85%), Architecture (85%)

Fix: Use mv instead:

mv "$SESSION_COUNT_FILE" "$BATCH_IDS_FILE"

This is atomic on the same filesystem and eliminates the race window.

[dean0x]

CWD Encoding Inconsistency

Line 63 uses sed s|/|-|g to encode the CWD, but this diverges from the established pattern in background-learning and background-memory-update which use sed s|^/|| | tr / -. While both produce similar results for typical paths, the inconsistency is fragile and could break with path variations.

Flagged by: Consistency (95%), Architecture (85%)

Fix: Align with the existing pattern:

ENCODED_CWD=$(echo "$CWD" | sed s|^/|| | tr / -)
PROJECTS_DIR="$HOME/.claude/projects/-${ENCODED_CWD}"

This ensures consistency across all hook scripts that need to encode paths.

[dean0x]

Per-Line Subprocess Spawning in reinforce_loaded_artifacts

Lines 108-131 iterate through every line of learning-log.jsonl in a while read loop, spawning json_field (jq or node subprocess) 2-3 times per line. With 50+ observations, this creates 100-300+ subprocesses in the synchronous SessionEnd hook path, adding measurable latency (0.5-2s) to every session end.

Flagged by: Performance (92%), Complexity (85%)

Fix: Replace with a single-pass jq operation:

if [ "$_HAS_JQ" = "true" ]; then
  local slugs_regex=$(echo "$loaded" | tr '\n' |)
  jq -c --arg now "$now_iso" --arg slugs "$slugs_regex" '
    if .status == "created" and .artifact_path != "" then
      (.artifact_path | split("/") | if test("/commands/") then .[-1] | rtrimstr(".md") else .[-2] end) as $slug
      | if ($slug | test($slugs)) then .last_seen = $now else . end
    else . end
  ' "$learning_log" > "$temp_log"
fi

This reduces from N spawns to 1 and eliminates the blocking I/O.

[dean0x]

Duplicate Temporal Spread Calculation in process_observations

Lines 508-517 and 521-530 both compute FIRST_EPOCH and NOW_EPOCH identically for the same observation. This duplicates date parsing overhead and violates DRY - any fix to the date parsing logic must be applied twice, risking divergence.

Flagged by: Complexity (95%)

Fix: Extract a single check_temporal_spread() function:

check_temporal_spread() {
  local first_seen="$1"
  FIRST_EPOCH=$(date -j -f "%Y-%m-%dT%H:%M:%SZ" "$first_seen" +%s 2>/dev/null \
    || date -d "$first_seen" +%s 2>/dev/null \
    || echo "0")
  NOW_EPOCH=$(date +%s)
  SPREAD=$((NOW_EPOCH - FIRST_EPOCH))
}

Then call once and reuse SPREAD in both blocks.

[dean0x]

hasLearningHook Returns False for Legacy Stop Hook

This function only checks settings.hooks.SessionEnd for the new marker. Users who installed learning before this PR have the hook registered under settings.hooks.Stop with the legacy stop-update-learning marker. After upgrading the CLI (before running --disable && --enable), hasLearningHook() returns false, causing --status to show learning as disabled even though the (now-deprecated) Stop hook is still executing.

Flagged by: Regression (85%), Architecture (72%)

Fix: Either (a) have hasLearningHook also detect the legacy marker and show a "needs upgrade" state, or (b) have --enable auto-detect and upgrade the legacy hook in-place. Option (a) is clearer:

export function hasLearningHook(settingsJson: string): boolean {
  const settings: Settings = JSON.parse(settingsJson);
  const hasSessionEnd = settings.hooks?.SessionEnd?.some(h => h.includes(HOOK_MARKER));
  const hasLegacyStop = settings.hooks?.Stop?.some(h => h.includes(LEGACY_HOOK_MARKER));
  return hasSessionEnd || hasLegacyStop; // Return true for either
}

export function getLearningStatus(): string {
  if (hasSessionEnd) return "enabled";
  if (hasLegacyStop) return "needs upgrade (legacy Stop hook). Run: devflow learn --disable && devflow learn --enable";
  return "disabled";
}

[dean0x]

addLearningHook Does Not Clean Up Legacy Stop Hook

When users run devflow learn --enable, this function only adds the new SessionEnd hook. It does not remove the legacy Stop hook that may still exist from pre-Wave-2 installations. Users who upgrade by running --enable will end up with both hooks registered, wasting a hook invocation on every session stop (the legacy Stop hook now just exits immediately).

Flagged by: Architecture (83%), Consistency (85%)

Fix: Have addLearningHook clean up the legacy hook first:

export function addLearningHook(settingsJson: string, devflowDir: string): string {
  // First, clean up any legacy Stop hook
  let cleaned = removeLearningHook(settingsJson);
  const settings: Settings = JSON.parse(cleaned);
  
  // Now add the new SessionEnd hook
  if (!settings.hooks) settings.hooks = {};
  if (!settings.hooks.SessionEnd) settings.hooks.SessionEnd = [];
  
  const hookPath = path.join(devflowDir, "hooks", HOOK_FILE);
  if (!settings.hooks.SessionEnd.includes(hookPath)) {
    settings.hooks.SessionEnd.push(hookPath);
  }
  
  return JSON.stringify(settings);
}

This makes --enable self-upgrading for existing users.

[dean0x]

Code Review Summary: Wave 2 Quality Issues (60-79% Confidence)

This PR introduces several consistency and style issues beyond the inline comments already posted. While individually minor, they should be addressed to maintain code quality:

Consistency Issues (80-85% confidence)

1. Log Timestamp Format Inconsistency (scripts/hooks/session-end-learning:55)

   • session-end-learning uses date '+%H:%M:%S' (HH:MM:SS local time)
   • background-learning uses date -u '+%Y-%m-%dT%H:%M:%SZ' (ISO 8601 UTC)
   • Since both write to the same log file, entries will be inconsistently formatted
   • Fix: Use ISO 8601 UTC format in both scripts for consistency

2. Conditional Logging Inconsistency (scripts/hooks/session-end-learning:53)

   • session-end-learning wraps logging in if [ "$DEBUG" = "true" ] guard
   • background-learning logs unconditionally
   • Creates confusing developer experience where background logs appear but trigger hook is silent
   • Fix: Make both conditional on DEBUG (preferred) or both unconditional

3. Sourcing Syntax Inconsistency (scripts/hooks/session-end-learning:21)

   • New hook uses . "$SCRIPT_DIR/json-parse" (POSIX dot syntax)
   • All other hooks use source "$SCRIPT_DIR/json-parse" (bash syntax)
   • Fix: Use source for consistency with existing patterns

4. Shell Strictness Inconsistency (scripts/hooks/session-end-learning:12)

   • New hook uses set -euo pipefail
   • All other hooks use set -e only
   • Stricter flags could cause unexpected failures if upstream scripts rely on different semantics
   • Fix: Use set -e to match codebase convention

5. Missing disown After Background Spawn (scripts/hooks/session-end-learning:200)

   • New hook spawns with nohup bash ... & but omits disown
   • Existing stop-update-memory uses nohup ... & && disown
   • Background job remains in job table unnecessarily
   • Fix: Add disown after background process spawn for consistency

Documentation Issues (90% confidence)

file-organization.md Not Updated (docs/reference/file-organization.md:50,157)

• Still references stop-update-learning as the learning hook
• Still says "Stop hook" instead of "SessionEnd hook"
• Creates active code-comment drift with actual implementation
• Fix: Update both references to session-end-learning and SessionEnd event type

Security/Robustness Issues (80-82% confidence)

1. ART_DESC Unescaped in YAML (scripts/hooks/background-learning:632,640)

   • Model-generated descriptions interpolated directly into YAML frontmatter
   • If model returns description with " or YAML-significant characters, frontmatter breaks
   • Fix: Escape quotes before interpolation

2. ART_NAME Sanitization Incomplete (scripts/hooks/background-learning:592)

   • Path traversal sanitization only strips / and .. pairs
   • Doesn't handle consecutive .. (e.g., .... reduces to .. after one pass)
   • Allows spaces, backticks, and shell metacharacters in paths
   • Fix: Use allowlist approach matching naming rules (kebab-case alphanumerics only)

3. Session ID Validation Missing (scripts/hooks/session-end-learning:162)

   • SESSION_ID from hook JSON appended directly to counter file without validation
   • While source is trusted (Claude runtime), defense-in-depth check is missing
   • Malformed session ID with newlines could inflate batch count
   • Fix: Validate format before appending

Performance Issue (90% confidence)

Per-Line Subprocess Spawning in extract_batch_messages (scripts/hooks/background-learning:166-177)

• Spawns json_extract_messages subprocess per user message line
• With 3 sessions × 50 messages = 150 subprocess spawns
• Extends background lock hold time
• Fix: Use single-pass jq command to extract all messages in one invocation

---

Overall Assessment

Blocking Issues: 8 fixed via inline comments (race condition, CWD encoding, subprocess spawning, temporal spread duplication, hasLearningHook legacy detection, addLearningHook cleanup)

Remaining Issues: The 12 items above should be addressed before merge. Most are quick fixes.

Recommendation: CHANGES_REQUESTED with focus on:

1. file-organization.md documentation update (must-do for accuracy)
2. Log timestamp/conditional logging consistency
3. ART_NAME sanitization improvement

---

Claude Code Review | 2026-03-25