Repository navigation
Conversation
- use llm as fallback for skill selector - only filter skills that do not have keywords and pattern
There was a problem hiding this comment.
Code Review
This pull request introduces an LLM-based fallback mechanism for skill selection, triggered when explicit mentions or deterministic keyword matching do not yield results. It includes logic for candidate filtering, context token budget management, and cost tracking for the fallback LLM call. The dispatcher was also updated to ensure correct message context is used during approval-resume flows. Review feedback suggests optimizing the implementation by using a cheaper LLM model for the selection task and utilizing standard JSON serialization instead of pretty-printing to reduce token overhead.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…mpts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Nice improvement for the v1 path. One thing to call out: this is still v1-only, since engine v2 does skill selection in If we want parity, we should mirror the same behavior in v2 as a follow-up: keep explicit Otherwise we’ll have different skill-selection behavior between v1 and v2 during the migration. |
You are correct! we should keep the same logic in v2 engine. but im not familiar with python and still dont know how to test v2 engine. i will dig into the code base with help of AI. and try to impl it. it would be great if some would help! |
…ction Co-authored-by: Copilot <copilot@github.com>
* fix: disable canara issues creation * ci: re-trigger regression-test-check after adding skip-regression-check label
|
@serrrfirat v2 engine part is finished and tested manually, please review the change |
|
Thank you for taking the time to contribute this PR—we genuinely appreciate the work and context you added. IronClaw is now consolidating development around the Reborn architecture and the upcoming This is an architecture-transition decision, not a reflection on the quality of your contribution. We would be very happy to see you contribute to Reborn. If you would like to revisit this idea in the current architecture, please reference this PR so the original work and discussion remain connected; maintainers can help identify the owning Reborn crate and the appropriate integration-test seam. Thank you again, and we hope to work with you on Reborn. |
we are trying to use skills from clawhub and it do not have activation keyworkds, this pr will try to select these skills using llm as a fallback
Summary
Change Type
Linked Issue
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildcargo test --features integrationif database-backed or integration behavior changedreview-prorpr-shepherd --fixwas run before requesting reviewSecurity Impact
Database Impact
Blast Radius
Touches
src/llm/reasoning.rs(return type ofselect_skill_names_with_llm),src/agent/agent_loop.rs(LLM fallback candidate filter + cost recording),and test fixtures in
src/agent/dispatcher.rs. No change to the happy pathfor explicit or deterministic skill selection.
Rollback Plan
Revert the three files changed in this PR. No schema or config changes involved.
Review Follow-Through
The
fallback_candidatesfilter now excludes any skill that has at least onekeyword, pattern, or tag. If a skill is intended to be description-only (no
activation criteria), it will still reach the LLM fallback. Reviewers should
confirm this is the desired boundary.
Review track: