Fold skill learning into skills - #5866
serrrfirat wants to merge 1 commit into
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE 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 |
github-actions
Bot
added
scope: docs
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consolidates the codebase by removing the separate ironclaw_skill_learning crate and integrating its pure domain logic into a new learning module within the ironclaw_skills crate. All workspace dependencies, module imports, agent documentation, and parity documentation have been updated to reference ironclaw_skills::learning instead of the deprecated crate. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 1 | 1 | 667f9c743eae |
Head: 667f9c743eae9df8378cbc74c1760e005f0eb4ae
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No blocking regression found in the skill-learning move. One secondary lockfile still references the removed crate and should be refreshed.
Findings
Blocking: 0 / Notes: 1
Non-blocking notes (1)
1. 💬 [LOW] Refresh the latency runner lockfile after removing the crate
Location: harness/latency/runner/Cargo.lock:3192
This separate Cargo.lock still records ironclaw_reborn_composition as depending on ironclaw_skill_learning, even though the PR removes that path dependency and deletes the crate manifest. Running the latency runner with a locked build, or running it and expecting a clean worktree afterward, will require Cargo to rewrite this lockfile. Regenerate harness/latency/runner/Cargo.lock so it matches the new dependency graph.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/failed/superseded state while reviewers run.
Inline review fallback
Inline comment projection fell back to a body-only PR Review because GitHub rejected the inline payload.
Reason: Unprocessable Entity: "Path could not be resolved" - https://docs.github.com/rest/pulls/reviews#create-a-review-for-a-pull-request
IronLoop preserved the inline review comment payloads below instead of dropping them.
Inline fallback 1: harness/latency/runner/Cargo.lock:3192
IronLoop reviewer: [LOW] Refresh the latency runner lockfile after removing the crate
This separate Cargo.lock still records ironclaw_reborn_composition as depending on ironclaw_skill_learning, even though the PR removes that path dependency and deletes the crate manifest. Running the latency runner with a locked build, or running it and expecting a clean worktree afterward, will require Cargo to rewrite this lockfile. Regenerate harness/latency/runner/Cargo.lock so it matches the new dependency graph.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/failed/superseded state while reviewers run.
|
🚅 Deployed to the ironclaw-pr-5866 environment in ironclaw-ci-preview
|
|
Superseded by #5874. |
This branch was successfully deployed
1 participant
Summary
W2.2 of the Reborn crate-count reduction folds
ironclaw_skill_learningintoironclaw_skills.This removes one workspace crate while preserving the learning boundary:
ironclaw_skills::learningcrates/ironclaw_skills/prompts/ironclaw_skills::learningironclaw_skill_learningpackage/dependency edgeWhy
ironclaw_skill_learningwas a good temporary extraction while the feature was taking shape, but its permanent owner is the skills domain. The code already validated throughironclaw_skills::parse_skill_md, and the only production caller was composition.This fold keeps the important separation intact:
ironclaw_skills::learningowns pure prompts, parse/validate logic, and theSkillInferencePortabstractionironclaw_reborn_compositionstill owns concrete LLM inference adapters, transcript reads, scoped writes, safety scanning, and UI notificationsSo we reduce crate count without moving runtime concerns into the skills crate.
Stack
Base: #5852 (
codex/reborn-layer-allowlist-w0-main)This is intentionally a small W2 fold PR. It should be retargeted to
mainafter W0 lands.Validation
cargo test -p ironclaw_skills -p ironclaw_architecturecargo check -p ironclaw_reborn_composition --features root-llm-providerironclaw_reborndead-code paths and one existing composition unused importcargo fmt --checkgit diff --checkcargo metadata --format-version 1 --no-deps | jq '[.packages[] | select(.name=="ironclaw_skill_learning")] | length'->0