Skip to content

fix: resolve skills not exposed to agents and LogLevel enum errors#3209

Open
JasonW404 wants to merge 2 commits into
developfrom
fix/skills-not-exposed-and-loglevel-error
Open

fix: resolve skills not exposed to agents and LogLevel enum errors#3209
JasonW404 wants to merge 2 commits into
developfrom
fix/skills-not-exposed-and-loglevel-error

Conversation

@JasonW404

@JasonW404 JasonW404 commented Jun 9, 2026

Copy link
Copy Markdown
Member

Summary

Fix two critical runtime issues that prevented skills from being exposed to agents and caused agent execution failures.

Issues Fixed

1. Skills Not Exposed to Agents (skills_count=0)

Root Cause: The TokenBudgetStrategy was silently dropping the SkillsComponent because the formatted skills description (with the 6-step usage process) exceeded the 1000-token budget allocated to the skills component type.

Fix: Increased the skills token budget from 1000 to 4000 tokens in summary_config.py to accommodate the verbose skill usage instructions (~2500-3500 chars).

Additional Fix: Added missing skills sections to English prompt templates (manager_system_prompt_template_en.yaml and managed_system_prompt_template_en.yaml) to provide a fallback path when the component-based assembly is not used.

2. LogLevel Enum AttributeError

Root Cause: Code referenced LogLevel.WARNING which doesn't exist in the smolagents LogLevel enum (only has OFF, ERROR, INFO, DEBUG).

Fix: Replaced LogLevel.WARNING with LogLevel.ERROR at two locations in core_agent.py (lines 417 and 804).

Changes

  • sdk/nexent/core/agents/summary_config.py: Increased skills budget from 1000 to 4000 tokens
  • sdk/nexent/core/agents/core_agent.py: Fixed LogLevel.WARNING → LogLevel.ERROR (2 occurrences)
  • backend/prompts/manager_system_prompt_template_en.yaml: Added skills sections
  • backend/prompts/managed_system_prompt_template_en.yaml: Added skills sections
  • backend/agents/create_agent_info.py: Added diagnostic logging and improved exception handling
  • test/backend/utils/test_context_component_types.py: Added 38 comprehensive tests

Testing

All 104 tests pass (38 backend + 66 SDK), zero regressions.

Verification

After applying these fixes:

  • Skills are now visible in agent system prompts
  • Agents can call read_skill_md(), run_skill_script(), and other skill-related tools
  • No more AttributeError: WARNING crashes during agent execution
image

@JasonW404 JasonW404 requested a review from WMC001 as a code owner June 9, 2026 08:14
Copilot AI review requested due to automatic review settings June 9, 2026 08:14
@JasonW404 JasonW404 requested a review from Dallas98 as a code owner June 9, 2026 08:14
@JasonW404 JasonW404 force-pushed the fix/skills-not-exposed-and-loglevel-error branch from be13985 to 1c65925 Compare June 9, 2026 08:20

This comment was marked as resolved.

@JasonW404 JasonW404 force-pushed the fix/skills-not-exposed-and-loglevel-error branch from 1c65925 to 24e0752 Compare June 9, 2026 08:28
@JasonW404 JasonW404 marked this pull request as draft June 9, 2026 08:54
- Fix LogLevel.WARNING AttributeError by replacing with LogLevel.ERROR
  (smolagents LogLevel enum only has OFF/ERROR/INFO/DEBUG, no WARNING)
  at core_agent.py lines 417 and 804

- Increase skills token budget from 1000 to 4000 in summary_config.py
  to accommodate the verbose 6-step skill usage process (~2500-3500 chars)
  that was being silently dropped by TokenBudgetStrategy

- Add skills sections to English prompt templates (manager + managed)
  mirroring the Chinese template structure with <available_skills> block
  and skill usage requirements section

- Add diagnostic logging in create_agent_info.py and core_agent.py to
  track skills count and component assembly for debugging

- Improve exception handling in _get_skills_for_template() with ERROR
  level logging and full stack trace for better observability

- Add comprehensive test suite (test_context_component_types.py) with
  38 tests covering component types, assembly validation, and semantic
  equivalence between Jinja2 templates and component assembly path

All 104 tests pass (38 backend + 66 SDK), zero regressions.
@JasonW404 JasonW404 force-pushed the fix/skills-not-exposed-and-loglevel-error branch from 24e0752 to f8797b0 Compare June 9, 2026 08:55
@JasonW404 JasonW404 marked this pull request as ready for review June 9, 2026 09:16
@wuyuanfr

wuyuanfr commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the thorough root-cause analysis and the detailed PR description. A few observations after reviewing the diff:

  1. Skills budget bump (1000 → 4000) — correct fix, but worth a guard
    Increasing skills budget from 1000 to 4000 in summary_config.py fixes the immediate symptom (TokenBudgetStrategy silently dropping SkillsComponent). However, if skill descriptions grow further (more verbose usage instructions, longer reference chains), the same silent-drop can happen again. Two suggestions:
  • Add a log warning when a strategy drops a component — I see 790d3ea introduced chore: log when a ContextStrategy drops a component in a previous commit, but it doesn't seem to be active in the current develop code path. Activating this would make future budget overflows visible immediately.
  • Consider making TokenBudgetStrategy emit a fallback instead of silently omitting — e.g., if the formatted description exceeds budget, truncate with a "skill description truncated" marker rather than dropping the whole section. This preserves the skill name/visibility while respecting the budget.
  1. EN template skills section — good addition, but EN/ZH asymmetry widening
    Adding the skills sections to the EN templates (manager_system_prompt_template_en.yaml, managed_system_prompt_template_en.yaml) is the right thing to do. However, the EN templates now diverge further from ZH in structure:
  • EN templates still don't have user_id in the header line
  • ZH templates still have 现在是{{time}}... in the header (unless another PR removes it separately)
    Just flagging this so it's tracked — not a blocker for this PR.
  1. build_skills_usage_component return type change (SystemPromptComponent → SkillsComponent) — 👍
    Changing the return type from SystemPromptComponent to SkillsComponent is a meaningful improvement. SkillsComponent carries the structured skills list and formatted_description, which gives TokenBudgetStrategy (or any future strategy) better visibility into what it's selecting, rather than just seeing opaque content text. This is the right direction.
  2. Knowledge base moved from _format_tools_description → build_knowledge_base_component — 👍
    Extracting KB summary from the tools section into its own KnowledgeBaseComponent is cleaner. It means KB content is independently budgeted and can be selected/dropped by strategy without affecting tool visibility. The language-aware prefix ("knowledge_base_search 工具只能使用以下知识库索引...") is also correctly moved here.
  3. build_available_resources_header_component (new) — minor concern about priority=55
    The new "Available Resources" header component has priority=55. The current ordering is:
    Header(100) → Memory(90) → Duty(80) → Skills(70) → ExecutionFlow(65) → AvailableResourcesHeader(55) → Tools(50) → KB(10) → ...
    This makes sense for manager agents ("你只能使用以下资源..."). But for managed agents, the header is just "### 可用资源" with no restrictive preamble. Is the intent that managed agents should also see a numbered resource list (1. Tools, 2. Agents, 3. Skills)? If yes, the numbering logic in _format_tools_description, _format_managed_agents_description, and _format_skills_usage_requirements now correctly adds 1./2./3. prefixes when is_manager=True. But for managed agents, these sections would appear without numbering — which may be inconsistent with the "Available Resources" heading. Worth a quick check.
  4. Nit: indentation mix in manager_system_prompt_template_en.yaml
    Around line 222, the {%- if not managed_agents...} block shifted from 5-space to 6-space indent. This is cosmetic but will render with inconsistent whitespace in the Jinja output. Might want to normalize.

Overall: The core fixes (budget increase, LogLevel enum, skills in EN templates) are correct and necessary. The structural refactoring (KB extraction, SkillsComponent type change, resources header) is going in the right direction. The above items are suggestions, not blockers.

…fault

- Add atomic replace_components() method to ContextManager to prevent
  race conditions when swapping components on conversation-level CM
- Fix run_agent.py to re-register components on surviving CM after
  overwrite (both MCP and non-MCP paths)
- Guard CM creation in nexent_agent.py with enabled check to avoid
  creating useless CM when context management is disabled
- Change enable_context_manager default from False to True
- Fix numbering consistency: tools and skills always show 1./3. prefix
- Fix indentation in manager_system_prompt_template_en.yaml (6→5 spaces)
- Add tests for replace_components() and component survival after overwrite
@JasonW404

Copy link
Copy Markdown
Member Author

Thanks for the thorough review! Addressing your points:

Point 1 (Log warning when strategy drops component): The logging is already active in the current code. See sdk/nexent/core/agents/agent_model.py:478-484:

logger.warning(
    "TokenBudgetStrategy dropped component type=%s priority=%d "
    "tokens=%d reason=%s (total %d/%d, type %d/%d)",
    comp.component_type, comp.priority, comp_tokens, reason,
    total_tokens, token_budget,
    current_type_total, comp_budget,
)

This logs whenever a component is dropped due to budget pressure, identifying whether it was the global or per-type budget that tripped.

Point 5 (Priority=55 concern for managed agents): Fixed in latest commit. Tools and skills now always show numbering (1. Tools, 3. Skills) regardless of is_manager flag, ensuring consistency between manager and managed agents.

Point 6 (Indentation nit): Fixed in latest commit. Lines 225-227 in manager_system_prompt_template_en.yaml now use 5-space indentation to match the surrounding template structure.

Additional fixes in this commit:

  • Resolved the dual ContextManager bug where components were registered on a discarded CM
  • Changed enable_context_manager default from False to True
  • Added atomic replace_components() method to prevent race conditions

Points 2-4 noted, thanks for the positive feedback on the structural improvements!

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.

3 participants